-
-
Notifications
You must be signed in to change notification settings - Fork 1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
[ENH] HeatMap: Allow setting the center when using diverging palettes #4809
Conversation
ad0f944
to
984f1e1
Compare
Codecov Report
@@ Coverage Diff @@
## master #4809 +/- ##
==========================================
- Coverage 84.01% 84.01% -0.01%
==========================================
Files 281 276 -5
Lines 56899 56140 -759
==========================================
- Hits 47805 47165 -640
+ Misses 9094 8975 -119 |
This seems to work. |
I wanted to minimize the height, which is already large. The benefit of having a disabled checkbox would be to let the user know it is there, but it still wouldn't tell her how to enable it. What do you say about adding a hint, like in, say Confusion, that shows the first time the user opens the widget? Something like "For data with a 'base value', choose a diverging palette and set its center."
I wanted the opposite effect. :) I wanted to "tie" this option to the palette by aligning it to the right and putting it as close to palette as I could. If this intention is not clear, I should indeed align it to the left, like normal options. Summary: I would keep hiding, but add a hint. I will align the checkbox to the left. |
984f1e1
to
cb05816
Compare
return self.__center | ||
|
||
def setCenter(self, center: float) -> None: | ||
def setCenter(self, center: Union[float, bool]) -> None: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Optional[float]?
Also
centerChanged = Signal(float)
needs to be changed to centerChanged = Signal(object)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not really; Optional[float]
would be equivalent to Union[float, None]
, while this was really Union[float, bool]
.
Terrible design. Now constructor gets either a float
or None
. In the latter case there is no centering, and it can't be re-enabled by changing center
from None
to float
.
dae3822
to
3b40630
Compare
If the Center at appears only for certain palettes then it is probably not that odd if it is located on the right, close to the palette selection box. (So left or right would be fine by me then.) |
d790f5f
to
ff1841a
Compare
OK, I prefer it on the right side. Moving back. |
Issue
Resolves #4789.
Includes