REF: Add tests.groupby.methods
State: closed

- [ ] closes #xxxx (Replace xxxx with the GitHub issue number)
- [ ] [Tests added and passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#writing-tests) if fixing a bug or adding a new feature
- [ ] All [code checks passed](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#pre-commit).
- [ ] Added [type annotations](https://pandas.pydata.org/pandas-docs/dev/development/contributing_codebase.html#type-hints) to new arguments/methods/functions.
- [ ] Added an entry in the latest `doc/source/whatsnew/vX.X.X.rst` file if fixing a bug or adding a new feature.

Is this a bad idea? Just moving around code without much value? It has a similar structure to `tests.frame.methods`. The end goal is to have `test_function.py` completely removed.

Labels:
name: Refactor
description: Internal refactoring of code
name: Testing
description: pandas testing functions or related to the test suite
name: Groupby
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
I think this is a good idea. Always good to have tighter scoped test organization
In general, I like splitting big files, however the downside is that when trying to only run 1 test by hand, the pytest command becomes longer.

In this case, I don't think its worth it if there's just 1 test in a file.

Maybe we can split the ``test_function.py`` file into a file for reductions and another file for transformations instead?
> In this case, I don't think its worth it if there's just 1 test in a file.

That isn't going to be the final state - but I didn't want to put too much into a PR. I moved the ones that were clearly meant just to test the given method from `test_function.py`. There will be more.

> Maybe we can split the test_function.py file into a file for reductions and another file for transformations instead?

We already have method-specific files for some reductions and transformations. Are you suggesting to consolidate all of the existing ones into these files? E.g.

 - nunique
 - quantile
 - rank
 - size
 - skew
 - min_max
 - nth
 - any_all
 - shift_diff
> > In this case, I don't think its worth it if there's just 1 test in a file.
> 
> That isn't going to be the final state - but I didn't want to put too much into a PR. I moved the ones that were clearly meant just to test the given method from `test_function.py`. There will be more.

Are you planning on moving other tests in addition to tests from ``test_function.py``?
(I think you've moved over half of the functions from ``test_function`` already)

IMO, if there are less than 10 tests in a file, I don't think it'd be worth it to make the new file in general.

> 
> > Maybe we can split the test_function.py file into a file for reductions and another file for transformations instead?
> 
> We already have method-specific files for some reductions and transformations. Are you suggesting to consolidate all of the existing ones into these files? E.g.
> 
> * nunique
> * quantile
> * rank
> * size
> * skew
> * min_max
> * nth
> * any_all
> * shift_diff

Yeah, I think you already do that for the ``cum*`` methods (I saw you put them in ``test_cum.py``).

This might make the PR too big though. Some of those files are also really large for some reason (at least ``min_max`` can be improved with more parameterization).
I don't think line or test counts per file should be our primary concern. To me, the main thing I want to make easy is the scenario "I'm a new contributor, I am fixing a behavior, where do I put a test?". For this, I think we should prefer a more consistent test organization rather than sacrificing layout for grouping a sufficient number of tests to satisfy some limit.

That said, having too many files in the test suite can definitely be detrimental. I don't think this structure approaches that amount, but that is quite opinionated (as is the rest of this comment :wink: ).
> I don't think line or test counts per file should be our primary concern. To me, the main thing I want to make easy is the scenario "I'm a new contributor, I am fixing a behavior, where do I put a test?". For this, I think we should prefer a more consistent test organization rather than sacrificing layout for grouping a sufficient number of tests to satisfy some limit.

The concern for me is typing out the extra characters in the path to the test file when I want to run a single test.
e.g.
``pytest pandas/tests/groupby/test_function.py -v -k ...`` -> ``pytest pandas/tests/groupby/methods/test_foo.py -v -k ...``
(if there's only a couple of tests in that file, then it isn't worth the annoyance for me esp. if I break multiple tests across files and have to type a long path a bunch of times)

This isn't a very strong opinion, though, and the current version in the PR is definitely better than what we have in main.
> the main thing I want to make easy is the scenario "I'm a new contributor, I am fixing a behavior, where do I put a test?

+1.  This can be non-trivial for even experienced contributors.
Thanks @rhshadrach. This is definitely a net positive