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

Indicate color clipping and control clipping colors #686

Merged
merged 6 commits into from Oct 12, 2016

Conversation

Projects
None yet
3 participants
@philippjfr
Member

philippjfr commented May 12, 2016

As the title says, adds a plot option to extend the colorbar in a particular. Here's an example:

%%opts Image [colorbar=True]
hv.Layout([hv.Image(np.random.rand(10,10))(plot=dict(cbar_extend=opt))
           for opt in ['min', 'max', 'both']])

image

Can also consider automatically enabling this if your data falls outside the specified range but that's another discussion.

@jbednar

This comment has been minimized.

Member

jbednar commented May 12, 2016

I don't get it -- what does it mean to extend the colorbar? It doesn't seem extended, it just now has a pointy end. Do you mean, make the colorbar visually indicate that it extends beyond the area shown, without actually changing the colorbar or the colormapping? If so that's not what the option name or the name of this issue suggests, and seems very confusing.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 12, 2016

True, the name isn't great, it's what matplotlib uses.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 12, 2016

This would also be more useful with an option to supply different colors for values that are above and below the range (and for NaN values). We can brainstorm about a nice way to specify that.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 12, 2016

pointy_cbar sounds more descriptive to me! ;-p

@jbednar

This comment has been minimized.

Member

jbednar commented May 12, 2016

I'm not sure it makes sense to provide direct control over this option, in HoloViews. Seems like HoloViews should just automatically turn it on if the user has selected a colormap that ends up clipping at a particular end of the colorbar, so that the visual representation is a reliable indicator of something meaningful. As it is, there's no relation between whether the user has explicitly enabled a pointy colorbar and anything about the actual plot, so indeed Jean-Luc's suggestion of calling it pointy_cbar is accurate!

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 12, 2016

Using this to indicate clipping when clipping occurs does make a lot more sense. We would still need a parameter to turn this behaviour on and off though...

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 12, 2016

Also I assume one problem this this suggestion is we might not know if clipping has occurred without inspecting the data range first? Sounds tricky...you might have to compare the data min/max with the limits to know if it should be pointy or not.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 12, 2016

Sounds tricky...you might have to compare the data min/max with the limits to know if it should be pointy or not.

It might be fairly straightforward and the overhead wouldn't be huge, so might be worth doing. Coupled with a way to specify special colors for min, max and NaN values that would be a nice addition.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 12, 2016

It might be fairly straightforward and the overhead wouldn't be huge..

Sure. As long as there is a way to turn it off and keep the current behaviour in case it ever becomes a problem. I agree it would be a nice feature and I'm wondering if there might be a way to specify the NaN values and clipping colors at the same time...

@jbednar

This comment has been minimized.

Member

jbednar commented May 12, 2016

That makes sense: the user can optionally enable having pointy cbars indicate such clipping. That way, whenever there is such a pointy cbar, it does accurately reflect something about the current plot, rather than just being some meaningless style option. Yet those who don't ever want pointy cbars can turn them off.

@philippjfr philippjfr changed the title from Added plot option to extend the colorbar in matplotlib to Indicate color clipping and control clipping colors in matplotlib May 13, 2016

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

So since I actually needed this for some plots of mine I've gone ahead and implemented it. If you've got a better suggestion for the clipping_colors parameter please suggest it and tell me and I also still need a name for the option that turns the clipping indicators on and off in general.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 13, 2016

Apparently ('w', 1) is used because the colorbar code in matplotlib takes the color and a separate alpha. I would much prefer to pretend matplotlib is consistent and support standard matplotlib colors, including the RGBA tuples.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

Sure, shouldn't be hard.

@@ -497,8 +497,10 @@ class ColorbarPlot(ElementPlot):
colorbar = param.Boolean(default=False, doc="""
Whether to draw a colorbar.""")
cbar_width = param.Number(default=0.05, doc="""
Width of the colorbar as a fraction of the main plot""")
clipping_colors = param.Dict(default={'NaN': ('w', 1)}, doc="""

This comment has been minimized.

@jlstevens

jlstevens May 13, 2016

Member

I understand what I expect to happen to NaN values but an example of how to set the min and max colors would be nice (in the docstring).

This comment has been minimized.

@philippjfr

philippjfr May 13, 2016

Member

Agreed.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 13, 2016

Philipp tells me you would specify a max clipping like this:

{'min': 'r', 'Nan': 'w'}

Would setting these colors make the colorbar pointy as appropriate (if the data is clipped?). If so wouldn't an empty dictionary correspond to the current behaviour without pointy colorbars?

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

If so wouldn't an empty dictionary correspond to the current behaviour without pointy colorbars?

No, not specifying a special color just sets the min and max color in the cmap as the clipping color, as in the example above.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 13, 2016

So why not have something like:

cbar_extension = {'min': 'default', 'max':'default'}

To indicate pointy colorbars with the min/max of the colormap where you can then specify your own custom colors instead of 'default'?

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

Sounds good, if we can come up with a better name for the parameter.

@jbednar

This comment has been minimized.

Member

jbednar commented May 13, 2016

Maybe cbar_clip?

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 13, 2016

Given that colorbar is a parameter name, colorbar_clip would appear next to it. For that reason I might like the longer name.

@jbednar

This comment has been minimized.

Member

jbednar commented May 13, 2016

Sure, if "clip" is accurate as a description about what it does (which I've somewhat lost track of by this point. :-)

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

We already have cbar_width and cbar_padding so it should at least be consistent.

Edit: Of course we could decide to rename those as well.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 13, 2016

We already have cbar_width and cbar_padding so it should at least be consistent.

In that case we should alias colorbar with cbar to enable/disable color bars.

@philippjfr

This comment has been minimized.

Member

philippjfr commented May 13, 2016

I'd suggest just calling it clipping_colors or color_clipping. Adding the pointy bit to the color bar is just a secondary side effect. Even without the colorbar enabled this will control the out-of-range colors so it's not really a colorbar option.

Edit: I guess why I was hesitant about the {'max': 'default'} suggestion is for the same reason, this parameter doesn't necessarily have anything to do with the colorbar, so enabling it that way feels a bit weird.

@jlstevens

This comment has been minimized.

Member

jlstevens commented May 14, 2016

clipping_colors seems fine. So could you still use it to disable pointy colorbars?

@jbednar

This comment has been minimized.

Member

jbednar commented May 24, 2016

See #689 for proposed UI.

@philippjfr

This comment has been minimized.

Member

philippjfr commented Oct 7, 2016

Ready to merge once clipping color support is fixed in bokeh: bokeh/bokeh#5323

@philippjfr philippjfr changed the title from Indicate color clipping and control clipping colors in matplotlib to Indicate color clipping and control clipping colors Oct 7, 2016

@@ -748,6 +748,11 @@ class ColorbarPlot(ElementPlot):
location, orientation, height, width, scale_alpha, title, title_props,
margin, padding, background_fill_color and more.""")
clipping_colors = param.Dict(default={'NaN': (0, 0, 0, 1)}, doc="""
Dictionary to specify colors for clipped values, allows setting

This comment has been minimized.

@jlstevens

jlstevens Oct 7, 2016

Member

RGBA tuples are fine though are any other color specifications allowed? E.g hex strings?

This comment has been minimized.

@philippjfr

philippjfr Oct 7, 2016

Member

Yes, docstring should be updated to reflect that.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Oct 12, 2016

Looks good. Happy to merge when the tests pass.

@jlstevens

This comment has been minimized.

Member

jlstevens commented Oct 12, 2016

Tests have all passed. Merging.

@jlstevens jlstevens merged commit 3c29b0b into master Oct 12, 2016

4 checks passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
continuous-integration/travis-ci/push The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.01%) to 72.998%
Details
s3-reference-data-cache Test data is cached.
Details

@philippjfr philippjfr deleted the cbar_extend branch Oct 14, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment