test groupby.indices for multiple groupby and mix of types

[alexifm]

Creates tests for GH26859

• closes Groupby indices error with datetime categorical #26859
• tests added / passed
• passes black pandas
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

Does this warrant an entry in whatsnew?

[arw2019]

Thanks @alexifm for the PR!

Some comments. More generally the second function is a lot to take in, wondering if it can be simplified somehow

[alexifm]

Thanks @alexifm for the PR!

Some comments. More generally the second function is a lot to take in, wondering if it can be simplified somehow

yea, probably can just run the groupby on all the columns and remove the parametrization. I'd have to check again if there was anything special about 1 vs multiple fields in a groupby. Or perhaps split into two simpler tests?

[arw2019]

yea, probably can just run the groupby on all the columns and remove the parametrization. I'd have to check again if there was anything special about 1 vs multiple fields in a groupby.

Unless they're direct replicas of each other we likely want to test both

Or perhaps split into two simpler tests?

i'd say split into two tests

[jreback]

use self.assert_numpy_array_equal for the comparisions.

pls simply as much as possible.

[alexifm]

I don't follow. What is self in this? Also that's a comparison on the keys, which are single objects or tuples, depending on how many columns are in the groupby.

[jreback]

sorry, tm.assert_assert_numpy_array_equal

[rhshadrach]

Why use set? Is the order not expected to be the same?

[alexifm]

@arw2019

Unless they're direct replicas of each other we likely want to test both

The difference is whether the keys are tuples or not:

>>> df.groupby(['a', 'b']).indices
{('a', Timestamp('2018-01-01 00:00:00')): array([0]),
 ('b', Timestamp('2018-02-01 00:00:00')): array([1]),
 ('c', Timestamp('2018-03-01 00:00:00')): array([2])}

>>> df.groupby(['a']).indices
{'a': array([0]), 'b': array([1]), 'c': array([2])}

So nearly identical behavior but subtly different. I split the tests but can bring them back together.

[jreback]

these test are overly complicated. can you simplify this by hard-coding the expected index locations and just doing a single comparision at the end.

[rhshadrach]

@arw2019 is right, split into two tests instead of stacking two different tests within one. However, when the tests are nearly identical, they should be parametrized instead. I think that's the case here, see my question below.

[rhshadrach]

I think is_cat_dt can be removed here, can you check?

[rhshadrach]

If this can be removed, then I think these tests can be combined using @pytest.mark.parametrize

[rhshadrach]

Replace this with data dictionary instead, use list(data.keys()) where cols is used now.

[rhshadrach]

Why use set? Is the order not expected to be the same?

[jreback]

@alexifm can you merge master and update to comments

[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.

[jreback]

closing as stale. if you want to continue, pls ping and can re-open.