REF: Move generic methods to aggregation.py#30856
REF: Move generic methods to aggregation.py#30856jreback merged 16 commits intopandas-dev:masterfrom
Conversation
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
looks good, some added typing and fixups of the doc-strings would be good. ping on green.
we can do other things in followons (e.g. making actual logic changes).
pandas/core/aggregation.py
Outdated
| return aggspec, columns, col_idx_order | ||
|
|
||
|
|
||
| def _make_unique(seq): |
There was a problem hiding this comment.
can you make this name more verbose: _make_unique_kwarg_list
There was a problem hiding this comment.
sure, changed!
| @@ -633,28 +633,28 @@ def test_lambda_named_agg(func): | |||
|
|
|||
| class TestLambdaMangling: | |||
There was a problem hiding this comment.
move all of these tests to pandas/tests/test_aggregation.py
There was a problem hiding this comment.
just the tests where you are moving the code (e.g. the maybe_mangle.....) and such
There was a problem hiding this comment.
yeah, I was thinking it as a follow-up, but maybe good to have it in a go here! I moved all tests related to functions in aggregation to pandas/tests/test_aggregation.py
|
Hello @charlesdong1991! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2020-01-20 22:05:57 UTC |
|
lgtm. ping on green. |
|
thanks @charlesdong1991 |
4 participants
xref #29116
#29116 (review)
based on @jreback comment, this PR is a precursor PR for #29116 to make PR smaller and more readable