Fixed bug computing categorical datashader aggregates #2295
Conversation
|
Now added tests, ready to merge once passing. |
|
I've also pushed a fix for #2299 to this PR and added a unit test. |
holoviews/operation/datashader.py
Outdated
| if isinstance(agg_fn, ds.count_cat): | ||
| name = '%s Count' % agg_fn.column | ||
| vdims = [dims[0](column)] | ||
| name = '%s Count' % column |
There was a problem hiding this comment.
name = ('%s Count' % column) if isinstance(agg_fn, ds.count_cat) else column?
Personally, I don't like it when I see simple variable reassignments e.g x=y used in this way. Up to you, I won't hold the merge up because of this.
There was a problem hiding this comment.
There is more to the if/else block than just this single line.
There was a problem hiding this comment.
Suppose I see what you're getting at.
|
Happy to merge once you reply to my comment. I won't hold up the merge if you don't want to make the suggested change. |
|
Made the change, ready to merge once tests pass again. |
|
Tests are passing. Merging. |
|
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. |
Recently I added some validation to xarray Datasets that disallows coords (kdims) and data variables (vdims) with the same name, which was apparently being used by the datashader
aggregateoperation for categorical aggregates. This handles them correctly.