Groupby tests restructure #14817
Groupby tests restructure #14817
Conversation
|
@jreback i have created this pr for the groupby tests restructure, at this point i have only moved the group by filter tests to a new file. Wanted to confirm if this is how it needs to be done also for the review comments. |
|
you can name test_groupby_filters -> test_filters.py yes this is the idea |
Current coverage is 84.55% (diff: 100%)@@ master #14817 diff @@
==========================================
Files 144 144
Lines 51043 51043
Methods 0 0
Messages 0 0
Branches 0 0
==========================================
- Hits 43552 43159 -393
- Misses 7491 7884 +393
Partials 0 0
|
|
@aileronajay can you update a bit (IOW, split maybe a little more). ping on green and can merge right away to prevent merge conflicts. |
|
@jreback i do intend to pick this up from next week (just stuck with some stuff right now) |
aileronajay
force-pushed
the
groupby_test_restructure
branch
from
b4297c4 to
f6e1cda
Compare
|
@jreback i have further split the tests into aggregate and categorical tests. So now we have filters, aggregate and categorical tests in separate files. There is also the original file with the tests that were not moved to one of the new files |
|
looks like you have |
|
@jreback it was a duplicate, i have removed it now |
| 'E': np.random.randn(11), | ||
| 'F': np.random.randn(11)}) | ||
|
|
||
| def test_agg_api(self): |
There was a problem hiding this comment.
can you remove the _agg name from these tests (as its now redundant)
| result = self.df.groupby(cats).D.apply(get_stats) | ||
| self.assertEqual(result.index.names[0], 'C') | ||
|
|
||
| def test_apply_categorical_data(self): |
There was a problem hiding this comment.
same here on the naming (of course if it adds value when reading the test name, then leave it where it does).
| '75%', 'max'] * 4) | ||
| self.assert_index_equal(desc_result.index.get_level_values(1), exp) | ||
|
|
||
| def test_groupby_categorical_index(self): |
|
|
||
| tm.assert_index_equal(result.columns, cats) | ||
| tm.assert_categorical_equal(result.columns.values, cats.values) | ||
|
|
There was a problem hiding this comment.
so for the categorical ones, they are mostly ok
| assert_frame_equal( | ||
| grouped.filter(lambda x: x['A'].sum() > 10), expected) | ||
|
|
||
| def test_filter_out_all_groups(self): |
There was a problem hiding this comment.
i would remove _filter as appropriate
|
@jreback i had thought about removing the agg (and other identifiers) but then i wonder if leaving them in place is detrimental? |
|
@aileronajay so on second thought, let's do that in another PR. then the changes will be obvious. So just leave the split files here. ping on green. |
|
@jreback all tests are passing |
|
thanks @aileronajay ok, couple of followups. originally had 302 tests, now have 305, so a couple duped (not a big deal but if can fix would be great). move apply, transform, agg from test_groupby -> test_agg (IOW anything having to do with these) after that if you can find any more logical categories to split tests, pls post. |
|
@jreback i was thinking of moving all cython tests into another file, but the cython tests individually belong to a different category each |
closes pandas-dev#14813 Author: Ajay Saxena <[email protected]> Closes pandas-dev#14817 from aileronajay/groupby_test_restructure and squashes the following commits: 860574d [Ajay Saxena] removed duplicate file f6e1cda [Ajay Saxena] further split the tests 2cc0734 [Ajay Saxena] branched out the filter tests into a new file
closes pandas-dev#14813 Author: Ajay Saxena <[email protected]> Closes pandas-dev#14817 from aileronajay/groupby_test_restructure and squashes the following commits: 860574d [Ajay Saxena] removed duplicate file f6e1cda [Ajay Saxena] further split the tests 2cc0734 [Ajay Saxena] branched out the filter tests into a new file
closes #14813