Fix GridInterface multi-dimensional groupby #1130
Conversation
philippjfr
added
type: bug
philippjfr
changed the title
holoviews/core/data/grid.py
Outdated
|
|
||
| # Iterate over the unique entries applying selection masks | ||
| grouped_data = [] | ||
| for unique_key in zip(*util.cartesian_product(keys)): | ||
| for unique_key in zip(*(arr.flat for arr in util.cartesian_product(keys))): |
There was a problem hiding this comment.
The definition of cartesian_product(arrays) is simply np.broadcast_arrays(*np.ix_(*arrays)). I'm wondering if it might be useful behavior to always output an iterator over flattened/flat arrays...
There was a problem hiding this comment.
I should mention that I am assuming that we are almost always doing cartesian products over keys which we want to iterate over. I haven't checked where else cartesian_product maybe used... e.g elsewhere in the data API...
There was a problem hiding this comment.
Right, they're actually always flattened, I'll make that fix.
There was a problem hiding this comment.
Ah, I remember now the issue is the difference between flat and flatten. Often flat is enough other times you need to flatten to make a copy. I'll give the function a copy kwarg.
There was a problem hiding this comment.
Hmm, actually there is one usage where it's not flat. Should I also have a flat kwarg?
There was a problem hiding this comment.
Yes, a flat kwarg would be good and a copy kwarg also makes sense (should probably default to False).
|
Made one comment about |
philippjfr
force-pushed
the
grid_interface_multi_groupby
branch
from
adbf888
to
ade9734
Compare
|
Ready to merge. |
|
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. |
As @basnijholt originally reported there was an issue for multi-dimensional groupbys in the GridInterface. This PR fixes the bug and adds a unit test.