TST: organize and cleanup pandas/tests/groupby/test_aggregate.py #18931

alysivji · 2017-12-24T19:13:09Z

Module currently has tests in 2 classes (TestGroupByAggregate and TestGroupByAggregateCython). The remaining tests are not in a class.

Also made the following changes:

TestGroupByAggregate class

test_agg_must_agg

replaced pytest.raises with tm.assert_raises_regex

test_agg_apply_corner

made it more readable

test_agg_multiple_functions_too_many_lambdas

replaced pytest.raises with tm.assert_raises_regex

test_multi_function_flexible_mix

made it more readable

TestGroupByAggregateCython class

all test methods that contain cython in the test name

test_cython_agg_nothing_to_agg

replaced pytest.raises with tm.assert_raises_regex

test_cython_agg_return_dict

replaced self.df with df initialized inside function

All Others

test_agg_dict_renaming_deprecation

made it more readable

test_agg_nested_dicts

replaced pytest.raises with tm.assert_raises_regex
made it more readable

test_agg_item_by_item_raise_typeerror

replaced pytest.raises with tm.assert_raises_regex

test_agg_structs_series

made it more readable

codecov · 2017-12-24T20:24:43Z

Codecov Report

Merging #18931 into master will decrease coverage by <.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #18931      +/-   ##
==========================================
- Coverage   91.58%   91.58%   -0.01%     
==========================================
  Files         150      150              
  Lines       48972    48972              
==========================================
- Hits        44851    44849       -2     
- Misses       4121     4123       +2

Flag	Coverage Δ
#multiple	`89.94% <ø> (-0.01%)`	⬇️
#single	`41.72% <ø> (ø)`	⬆️

Impacted Files	Coverage Δ
pandas/util/testing.py	`84.74% <0%> (-0.22%)`	⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8433562...5d763ff. Read the comment docs.

jreback · 2017-12-26T22:04:54Z

thanks @alysivji

can you split things out to new files?

IOW each of these classes to a separate file, something like

pandas/tests/groupby/aggregate/test_cython (2nd one)
test_aggregate (1st class)
test_other (3rd one)

just to make this even more organized.

be sure to add a __init__.py in the sub-dir

alysivji · 2017-12-27T18:34:34Z

Split up the files
cleaned up the imports
changed all assert_frame/assert_series calls to tm.assert_*
more cleaning up tests to increase readability
rebased

@jreback lemme know if anything else needs to be done.

Thanks!

jreback

@alysivji looking good. some requests to parameterize. could completely eliminate the need for the setup method in aggregate/test_aggregate.py if you want to make those into fixtures instead. can do this in another PR if you want.

jreback · 2017-12-27T19:12:30Z

pandas/tests/groupby/aggregate/test_aggregate.py

+        expected = grouped.mean()
+        tm.assert_frame_equal(result, expected)
+
+    def test_aggregate_str_func(self):


can you parametrize this (on by_weekday and by_mwkday)

jreback · 2017-12-27T19:12:48Z

pandas/tests/groupby/aggregate/test_aggregate.py

+        df['E'] = ['a'] * len(self.df)
+        grouped = self.df.groupby('A')
+
+        # API change in 0.11


can remove this commented out

jreback · 2017-12-27T19:13:21Z

pandas/tests/groupby/aggregate/test_cython.py

+
+class TestGroupByAggregateCython(object):
+
+    def test_cythonized_aggers(self):


can you parameterize this

jreback · 2017-12-27T19:13:41Z

pandas/tests/groupby/aggregate/test_cython.py

+        tm.assert_series_equal(summed, expected)
+
+    def test__cython_agg_general(self):
+        ops = [('mean', np.mean),


can you parameterize

jreback · 2017-12-27T19:13:48Z

pandas/tests/groupby/aggregate/test_cython.py

+                exc.args += ('operation: %s' % op, )
+                raise
+
+    def test_cython_agg_empty_buckets(self):


jreback · 2017-12-27T19:16:43Z

cc @TomAugspurger I think we have to merge #18921 (when ready) before this (and rebase this on master)?

TomAugspurger · 2017-12-27T19:27:57Z

Yes it'd be nice to merge #18921 before this. Backporting can be tricky when files are split.

jreback · 2017-12-27T19:43:59Z

@alysivji but go ahead and make the changes as above when you can. we will be merging @TomAugspurger soon anyhow.

alysivji · 2017-12-27T21:57:45Z

@jreback Changes have been made. Also deleted the cython test class, not needed anymore since it's in a separate module.

I can rebase when @TomAugspurger 's changes get in.

Also started working on turning the test_aggregate.py setup into fixtures. Lots of moving pieces for one PR; will send a separate request once this is merged.

jreback

looks good
couple of comments - will let u know when to rebase

jreback · 2017-12-27T22:04:15Z

pandas/tests/groupby/aggregate/test_cython.py

+        result = op(grouped)['C']
+        if name in ['sum', 'prod']:
+            tm.assert_series_equal(result, exp)
+


yes u can collapse testit - just move everything up

jreback · 2017-12-27T22:05:11Z

pandas/tests/groupby/aggregate/test_cython.py

+    expected = df.groupby(labels).agg(targop)
+    try:
+        tm.assert_frame_equal(result, expected)
+    except BaseException as exc:


yes u can take this try/except out
it’s helpful when not parametrized but not now

jreback · 2017-12-27T22:05:32Z

pandas/tests/groupby/aggregate/test_aggregate.py

+                                              ['C', grouped['C'].mean()],
+                                              ['D', grouped['D'].sem()]]))
+            tm.assert_frame_equal(result, expected)
+


same - collapse this function

jreback · 2017-12-29T21:42:54Z

@alysivji if you want to rebase we can get this in.

alysivji · 2017-12-30T03:24:45Z

@jreback all green!

Will start working on refactoring the setup method to fixtures this weekend.

jreback · 2017-12-30T12:32:07Z

thanks @alysivji

would like to remove the imports inside functions as well (and consolidate at the top-level) on your next PR.

…das-dev#18931)

sinhrks added Clean Testing pandas testing functions or related to the test suite labels Dec 25, 2017

alysivji force-pushed the issue_18490 branch 2 times, most recently from 02ebbf0 to dfd192e Compare December 27, 2017 17:29

jreback requested changes Dec 27, 2017

View reviewed changes

alysivji added 12 commits December 29, 2017 18:40

Restructure tests

5efa7ec

Orgnaize new tests

0350e2d

Change pytest.raises to pandas test helper

3b5fc39

Replace pytest.raises with pandas test helper

5a25b06

Add vscode to gitignore

91d3fa4

Fix failing test

8521a8e

Split tests_aggregate.py tests into multiple files

93e7e38

Clean up imports

86b6bf9

Change all assert_frame/assert_series to tm.assert_

4ee4b6b

Boyscouting -- cleaning up code

ee35394

Parametrize tests and remove commented out code

d48a149

Delete cython class, all tests are grouped by module

1add9dd

alysivji force-pushed the issue_18490 branch from 5be4ae1 to bb23e45 Compare December 30, 2017 02:01

Collapse functions and remove try except blocks

5d763ff

alysivji force-pushed the issue_18490 branch from bb23e45 to 5d763ff Compare December 30, 2017 02:08

jreback added this to the 0.23.0 milestone Dec 30, 2017

jreback approved these changes Dec 30, 2017

View reviewed changes

jreback merged commit faeac49 into pandas-dev:master Dec 30, 2017

hexgnu pushed a commit to hexgnu/pandas that referenced this pull request Jan 1, 2018

TST: organize and cleanup pandas/tests/groupby/test_aggregate.py (pan…

a3e392f

…das-dev#18931)

alysivji deleted the issue_18490 branch January 5, 2018 14:24

alysivji mentioned this pull request Jan 5, 2018

TST: Finish groupby aggregate cleanup #19094

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

TST: organize and cleanup pandas/tests/groupby/test_aggregate.py #18931

TST: organize and cleanup pandas/tests/groupby/test_aggregate.py #18931

alysivji commented Dec 24, 2017

codecov bot commented Dec 24, 2017 •

edited

Loading

jreback commented Dec 26, 2017 •

edited

Loading

alysivji commented Dec 27, 2017

jreback left a comment

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback commented Dec 27, 2017

TomAugspurger commented Dec 27, 2017

jreback commented Dec 27, 2017

alysivji commented Dec 27, 2017

jreback left a comment

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback Dec 27, 2017

jreback commented Dec 29, 2017

alysivji commented Dec 30, 2017

jreback commented Dec 30, 2017


		class TestGroupByAggregateCython(object):

		def test_cythonized_aggers(self):

TST: organize and cleanup pandas/tests/groupby/test_aggregate.py #18931

TST: organize and cleanup pandas/tests/groupby/test_aggregate.py #18931

Conversation

alysivji commented Dec 24, 2017

TestGroupByAggregate class

test_agg_must_agg

test_agg_apply_corner

test_agg_multiple_functions_too_many_lambdas

test_multi_function_flexible_mix

TestGroupByAggregateCython class

test_cython_agg_nothing_to_agg

test_cython_agg_return_dict

All Others

test_agg_dict_renaming_deprecation

test_agg_nested_dicts

test_agg_item_by_item_raise_typeerror

test_agg_structs_series

codecov bot commented Dec 24, 2017 • edited Loading

Codecov Report

jreback commented Dec 26, 2017 • edited Loading

alysivji commented Dec 27, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 27, 2017

TomAugspurger commented Dec 27, 2017

jreback commented Dec 27, 2017

alysivji commented Dec 27, 2017

jreback left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jreback commented Dec 29, 2017

alysivji commented Dec 30, 2017

jreback commented Dec 30, 2017

codecov bot commented Dec 24, 2017 •

edited

Loading

jreback commented Dec 26, 2017 •

edited

Loading