IHasColormap and automatic Colorbar tick placement#1403
Conversation
This interface will be implemented by plottables containing a colormap to expose their colors/values easier to the colorbar class
Public properties were added to Colormap to define the value range defined by the color range
Now that the value range can be stored in the Colormap itself this interface does not need properties to hold these values
|
Hey @bclehmann, I know you've done a lot of work on heatmaps previously so I'd love your input on this strategy. I think it makes sense to connect heatmaps and colorbars this way, but another set of eyes on this would be great. I don't want to fall off the deep end of calculating ideal/round tick positions right now, but I'll probably create an issue to work toward that after this gets merged. |
bclehmann
left a comment
bclehmann
left a comment
There was a problem hiding this comment.
The main architecture looks good, I had just a couple notes on the details.
previously these were values stored in the Colormap itself
|
I see you merged it already @swharden, I was in the middle of a review. The comments were pretty small as it is. |
Dang, I'm really sorry for merging this in too quick @bclehmann - I had your initial review up on my other monitor and saw all the comments were ~6 hours old and I addressed/resolved all of them but didn't give you a chance to review the changes/comments and give additional feedback and final approval. I'll be more careful to move slower in the future 👍 Again, I really appreciate all your feedback here! |
|
All good, I see you addressed my comments in a later commit. There was a typo , I added a comment but I don't know if github notifies you if you comment on a commit. |
2 participants
This PR creates
IHasColormapwhich can be applied toPlottableobjects to expose their colormap/limits to theColorbarclass, allowing the two to be connected. Logic was also added to theColorbarclass to automatically generate ticks.Limitations: Ticks are generated for a target density (number of pixels per tick), evenly spaced-out between a min/max value, and are not intelligently chosen (e.g., round numbers). Tick placement/labeling can be refined in the future, perhaps in combniation with refactoring the
TickCollectionmodule which has been a to-do item for a long time #1028resolves #1362