Skip to content
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

Added support for categorical colormapping in bokeh backend #1137

Merged
merged 4 commits into from Feb 23, 2017

Conversation

@philippjfr
Copy link
Member

@philippjfr philippjfr commented Feb 15, 2017

Adds support for categorical colormapping on bokeh ColorbarPlot and implements it specifically for Points. We already have this support in matplotlib and bokeh now makes it easy with a CategoricalColorMapper. This is also already achievable with an NdOverlay of Points but this allows selections to work on a single datastructure which is a lot easier to deal with for streams.

%%opts Scatter [color_index=2 toolbar='above' legend_position='right' width=600] (cmap='Set1' size=10)
hv.Scatter([('Week %d' % (i%10), np.random.rand(), chr(65+np.random.randint(5)))
           for i in range(100)], kdims=['Date'], vdims=['r2', 'Group'])

screen shot 2017-02-15 at 12 33 09 am

@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Feb 20, 2017

Ready to merge when tests pass.

opts = dict(factors=factors)
if 'NaN' in colors:
opts['nan_color'] = colors['NaN']

This comment has been minimized.

@jlstevens

jlstevens Feb 20, 2017
Contributor

I am wondering if a chunk of this should be a utility, e.g these lines. Both self.clipping_colors and self.logz seem to be sensible argument for a general color mapping utility.

I'll leave it up to you but to me it seems like a fair chunk of code that doesn't have to be tied up in a plotting class.

This comment has been minimized.

@philippjfr

philippjfr Feb 20, 2017
Author Member

Yes, some of that can definitely be made into a utility.

This comment has been minimized.

@philippjfr

philippjfr Feb 23, 2017
Author Member

Wasn't really worth it factoring much of this out.

legend_specs = {'right': dict(pos='right', loc=(5, -40)),
'left': dict(pos='left', loc=(0, -40)),
'top': dict(pos='above', loc=(120, 5)),
'bottom': dict(pos='below', loc=(60, 0))}

def _process_legend(self, plot=None):

This comment has been minimized.

@jlstevens

jlstevens Feb 20, 2017
Contributor

I see this was moved from PointPlot and I expect it is more general here. That said, does this have anything to do with colormapping or is it just an unrelated improvement?

This comment has been minimized.

@philippjfr

philippjfr Feb 20, 2017
Author Member

No, it was moved from BarPlot. Basically it handles providing control over automatically created legends, which is a general concept. Since categorical colormappers can automatically create a legend it's useful to move this method up to LegendPlot and let PointPlot make use of it.

@jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Feb 20, 2017

Once my two comments are addressed, I'm happy to merge.

@philippjfr philippjfr force-pushed the categorical_cmapper branch from 61e9a57 to 2f4b51c Feb 23, 2017
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Feb 23, 2017

Factored out a tiny little function but the rest of it is just very closely tied to the signatures of the bokeh colormappers and won't benefit much from being separate utilities. This should be ready to merge now.

@philippjfr philippjfr force-pushed the categorical_cmapper branch from 2f4b51c to a8f0e1d Feb 23, 2017
@philippjfr
Copy link
Member Author

@philippjfr philippjfr commented Feb 23, 2017

Ended up factoring it out into a separate method. Ready to merge now.

@jlstevens
Copy link
Contributor

@jlstevens jlstevens commented Feb 23, 2017

A separate method also helps break up the block of code.

Looks good, merging.

@jlstevens jlstevens merged commit ed01c3c into master Feb 23, 2017
4 checks passed
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.008%) to 78.218%
Details
@philippjfr
s3-reference-data-cache Test data is cached.
Details
@philippjfr philippjfr deleted the categorical_cmapper branch Feb 27, 2017
@philippjfr philippjfr added this to the v1.7.0 milestone Feb 28, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants