BUG: handle multi-dimensional grouping better

[dimithras]

As described in issue #36158 multi-dimensional groups prepared with lambda or named function raise error on indices as isnan() is not defined for MultiIndex.

Grouping already has the check for MultiIndex on line 440, but it's not triggered on init when grouper is represented by lambda function and gets casted to MultiIndex later where MultiIndex stays as is.

Tested on the code provided in #36158, yet not sure if significant testing is needed as this is a relatively small change to code.
Grouper code has not been changed for a while, should be save to merge.
Tests for this particular issue provided.

Concerns: probably it will be better to make isinstance(grouper, MultiIndex): a function as after this change this piece of code occurs twice in init.
Those operate on different grouper object. First one is passed on init, second one is self, where grouper is changed from lambda to MultiIndex and needs further change added in PR.

• closes BUG: indices and get_group raise exception with GroupBy initialized using lambda or named function with tuple values as group names (Multi-dimensional groups) #36158
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[arw2019]

thanks for the PR!

Could you add tests illustrating the new behavior? That's the first thing we look for when reviewing

[dimithras]

Hi @arw2019 and thanks for initial review!
Updated PR, tests provided.

[dimithras]

All tasks successful, tests passed, ready for review.

[AlexanderCecile]

I thought I would mention this here, for visibility: The error also occurs with regular functions, not just lambda, see my comment on the issue.

[dimithras]

@AlexanderCecile thank you very much for your input, highly appreciated. Fair point, behavior is the same both for lambda and defined function.
I've just tested a function from your comment. Current solution also covers this case.

Nature of a bug:
With tuples in cells example using groupby by a column, grouper is initialized with actual values during grouper init (line 413), this way MultiIndex is casted to an array on line 440.
However if lambda or function is used, grouper is assigned lambda or named function respectively and line 440 is skipped as it does not see MultiIndex behind it.
Later on line 512 changes are triggered and line 518 assigns actual value (MultiIndex) instead of a function, however this case skips line 440 casting and bug appears.

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update or respond to this comment if you're still interested in working on this.

[rhshadrach]

@dimithras - Sorry, I missed your ping. If this is read for review, can you merge master and take this off draft status?

[dimithras]

Hi @rhshadrach , sorry for letting this hanged. I looked at the failing test and issues connected to it, I'm more or less sure it should not be a problem to remove the test. Yet would be better to have a green light from you or someone else more familiar with pandas dev than me.

I'll remove the failing test tomorrow (CEST) so that it will be ready to merge.

[dimithras]

Master merged with no conflicts, I also did a rebase to shrink the number of commits.
@rhshadrach got it out of draft for now. Let me know if something needs to be done on Github's failing tests, do not see any clue why this is happening.

[dimithras]

Converted to draft, test_counting.py test_groupby_count_dateparseerror is failing with same type error. Going to have a look at it.

[dimithras]

After merging with latest master the issue is gone without making changes to grouper.

@rhshadrach unfortunately _transform_index breaks the test on test_counting.py in test_groupby_count_dateparseerror where lambda function is passed all multiindex items one by one instead of tuples. Instead of 10 tuples to 10 rows it gets 20 items where multiindex has two values.

All changes reverted, PR left with added tests for original issue.

[rhshadrach]

py37_32bit failed from out of memory error, Windows py38_np18 has one unrelated failure:

FAILED pandas/tests/indexing/test_partial.py::TestPartialSetting::test_slice_irregular_datetime_index_with_nan

[rhshadrach]

/azp run

[azure-pipelines]

Azure Pipelines successfully started running 1 pipeline(s).

[rhshadrach]

Thanks for updating, changes look good.

[dimithras]

@jreback will you have a look if you have time?

[jreback]

thanks @dimithras