Support construct_color_bar for ImageStack glyph #13289
Conversation
Codecov Report
@@ Coverage Diff @@
## branch-3.3 #13289 +/- ##
===========================================
Coverage 92.44% 92.44%
===========================================
Files 315 315
Lines 20120 20120
===========================================
Hits 18599 18599
Misses 1521 1521 |
|
Thanks! Can you show the output from the example that now works? |
Here it is: 
The appearance is the same as it was originally, this PR just hooks up the convenience function to create the |
| @@ -216,7 +217,7 @@ def construct_color_bar(self, **kwargs: Any) -> ColorBar: | |||
| raise ValueError("expected text_color to be a field with a ColorMapper transform") | |||
| return ColorBar(color_mapper=text_color.transform, **kwargs) | |||
|
|
|||
| elif type(self.glyph) is Image: | |||
| elif type(self.glyph) in (Image, ImageStack): | |||
There was a problem hiding this comment.
Note that using type(self.glyph) instead of isinstance(self.glyph, ...) prevents sub-classes to be allowed in this branch. If this is intentional, then I would suggest explaining the reason.
There was a problem hiding this comment.
It is intentional in the sense that this code was already using type(self.glyph) and hence I have kept it on the basis of making the minimal set of changes to existing code to achieve what I want.
I am happy for it to remain using type or change it to use isinstance. Would you prefer the latter?
There was a problem hiding this comment.
I generally prefer such trivial things to be fixed in the same PR, rather than spending another PR on this, or worse tripping over such issue at some point in the future. If it was a more elaborate fix, then sure another PR would be preferred.
There was a problem hiding this comment.
Switched to using isinstance in commit e94dccd.
|
The CI failure on Windows is real, but unrelated to this PR (it's Chromium 115 changed something). |
ianthomas23
deleted the
ianthomas23/13288_image_stack_construct_color_bar
branch
* Support construct_color_bar for ImageStack glyph * Use isinstance instead of type
* Support construct_color_bar for ImageStack glyph * Use isinstance instead of type
* Support construct_color_bar for ImageStack glyph * Use isinstance instead of type
|
This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. |
This adds support for
construct_color_barforImageStackglyph. After this, the example in the referenced issue works.