-
-
Notifications
You must be signed in to change notification settings - Fork 402
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
Add consistent support for categorical axes in bokeh #1089
Conversation
8c318c3
to
87b0f51
Compare
This looks great, thanks! I second the vote for disabling webgl by default, until it becomes something fully supported and maintained and used consistently in Bokeh. |
""" | ||
Cleans the data before instantiating the datasource to handle | ||
categorical axes correctly. | ||
""" |
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.
What exactly do you mean by 'clean'?
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 the best name I agree, something like _categorize_data
would be more appropriate. Basically when overlaying on a categorical axis, column source data has to be converted to categories (which are simply the pretty printed representation of the value).
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.
I think calling it _categorize_data
and explicitly mentioning the string conversion/pretty printing in the docstring would be clearer.
I also agree. Being able to overlay points over a heatmap is something I've wanted in the past. Nice to know that it will soon be possible! This PR looks like it touches some core methods in the Bokeh backend. From what I've seen, the code looks ok but I would probably recommend merging this PR ASAP so we can test it properly. It might also be nice to add some notebook examples if we can find a suitable place to demonstrate this functionality. That might be tricky so how about adding some quick examples to contrib? |
Some example in the bokeh backend notebook might make sense. There's still work to be done to support this fully in matplotlib so for now that's the right place. |
I'll leave it up to you whether the examples go there or in contrib. Either way, I think some notebook examples would be helpful. |
1cd20cd
to
d3f4050
Compare
fb7805f
to
1aa18fe
Compare
Added a section to the notebook:
%%opts Points [size_index='z' tools=['hover']] HeatMap [toolbar='above' tools=['hover']]
points = hv.Points([(chr(i+65), chr(j+65), i*j) for i in range(10) for j in range(10)], vdims=['z'])
hv.HeatMap(points) * points
%%opts Overlay [show_legend=False height=400 width=600] ErrorBars (line_width=5) Scatter(alpha=0.2 size=6)
overlay = hv.NdOverlay({group: hv.Scatter(([group]*100, np.random.randn(100)*(i+1)+i))
for i, group in enumerate(['A', 'B', 'C', 'D', 'E'])})
errorbars = hv.ErrorBars([(k, el.reduce(function=np.mean), el.reduce(function=np.std))
for k, el in overlay.items()])
global_mean = hv.Text('A', 12, 'Global mean: %.3f' % overlay.dimension_values('y').mean())
errorbars * overlay * hv.Curve(errorbars) * global_mean |
@jlstevens, @jbednar Ready for final review. Would be very happy to see this go in. Not being able to update heatmaps with changing coordinates has annoyed me for a while, and making categoricals work more generally wasn't a major step from there. Unfortunately charts (Bars and BoxWhisker) still cannot be overlaid because of the way they are constructed. I could probably hack it, but really we should move to replace them at some point. |
The new notebook examples look good. The functionality in this PR is very useful and unless @jbednar has any additional comments, I am happy to see it merged. |
Happy to see it merged.
So in HoloViews, string == categorical? Various dataframe packages support treating other discrete types (e.g. integers) as categoricals as well, and under the hood typically use integers to represent the possible enumerated values. But from the above (particularly the 5593 commit) it looks like numbers are fine as the category labels, so I'm a bit confused about this bit of documentation. |
A HeatMap (and Bars/BoxWhisker if they could be overlaid) will force categorical axes even if the categories are integer/float types. Other Elements only treat string types as categorical. In future we discussed that a |
Makes sense. I'd vote for changing that bit of documentation so that it does not conflate string with categorical, if we can eventually distinguish those. I don't think we have to wait on #843, though, because whether a dimension is categorical seems very clearly semantic, to me, and not just a style option. So regardless of the outcome of #843, I would think this information belongs directly to the Dimension as a core property. |
Sure, but what else should I say?
Are you arguing I should add a
I do agree, it is a semantic thing and therefore would be reasonable to have directly on Dimension. |
You're the expert on what to say, but my guess would be what you said here in the chat: A number of Elements will also support categorical types as dimension values, including HeatMap, Points, Scatter, Curve, ErrorBar and Text types. Currently, whether a type is treated as categorical is determined partly by the Element type (e.g. all key dimensions on HeatMap and XXX types are treated as categorical), though string dimensions are considered categorical by all Element types. I'm not arguing for a categorical parameter as part of this PR, just arguing that it doesn't need to wait on the very-contentious #843 to be resolved. |
Okay, revised it.
Okay, and yes I agree, although @jlstevens wants to push forward on #843 soon. |
That's correct. I'm working on a Dimension tutorial which will discuss aliases. Once that is done I want to tackle #843 (I don't think it is contentious anymore!) at which point I will update the tutorial. I have no opinion on a |
It is a little bit, personally I still don't see strong arguments for it, but your strong opinion probably outweighs our relative indifference. |
At any rate, I don't think #843 needs to hold up this PR from being merged. |
Yes, ready now, PR build passed. |
Merged! |
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. |
Previously categorical axes weren't really handled in bokeh causing issues when trying to Overlay HeatMaps or other categorical Elements. We can now update categorical axes with new factors which makes it possible to animate HeatMaps with varying density:
We can also specify and overlay categorical curves and points:
And we can also overlay on top of a HeatMap:
And:
Note that
webgl
does not play well with this. There are now so many issues I've encountered with webgl that I would suggest disabling it by default.Elements that can use categorical axes: