Groupby tuples #18249
Groupby tuples #18249
Conversation
| # when MultiIndex, allow tuple to be a key | ||
| if not isinstance(key, (tuple, list)) or \ | ||
| (isinstance(key, tuple) and is_axis_multiindex): | ||
| if not isinstance(key, list): |
There was a problem hiding this comment.
can you add a nice comment here, explaining
pandas/core/generic.py Outdated
| values are used as-is determine the groups. A str or list of strs | ||
| may be passed to group by the columns in ``self`` | ||
| values are used as-is determine the groups. A label or list of | ||
| labels may be passed to group by the columns in ``self``. |
There was a problem hiding this comment.
maybe callout what a tuple does.
toobaz force-pushed the groupby_tuples branch from 1c78988 to 65f6da0 Compare 
| linting issue |
toobaz force-pushed the groupby_tuples branch from 65f6da0 to dafc838 Compare 
Codecov Report
@@ Coverage Diff @@ ## master #18249 +/- ## ========================================= Coverage ? 91.41% ========================================= Files ? 164 Lines ? 50090 Branches ? 0 ========================================= Hits ? 45790 Misses ? 4300 Partials ? 0
Continue to review full report at Codecov.
|
Codecov Report
@@ Coverage Diff @@ ## master #18249 +/- ## ========================================= Coverage ? 91.39% ========================================= Files ? 164 Lines ? 50095 Branches ? 0 ========================================= Hits ? 45783 Misses ? 4312 Partials ? 0
Continue to review full report at Codecov.
|
Fixed! ping @jreback |
| thanks! |
xref pandas-dev#17996 Author: Pietro Battiston <me@pietrobattiston.it> Closes pandas-dev#18249 from toobaz/groupby_tuples and squashes the following commits: dafc838 [Pietro Battiston] DOC: Clarification of groupby(by=) argument e0bdfa7 [Pietro Battiston] TST: Test for tuples in columns, fixes to previous tests 74f91e0 [Pietro Battiston] TST: Fix tests which used tuples to pass multiple keys 201a4fe [Pietro Battiston] BUG: Never interpret a tuple as a list of keys
2 participants
git diff upstream/master -u -- "*.py" | flake8 --diffSorry for having missed #17996, where this could have been discussed, but this said I don't think we should treat specially the case of a
MultiIndex(see the test I added with tuples as labels): we should just interpret what is not a list as a label, as the docs already state (although with some incoherence, which I tried to address).While I was at it, I fixed a typo which made a test inserted in #17996 trivial.