Add test for #32108 (error with groupby on series with period index) #33105

daxid · 2020-03-28T20:22:26Z

closes Series with NAMED period index raise error on groupby index.month (pandas 1.0 specific) #32108

pep8speaks · 2020-03-28T20:22:29Z

Hello @daxid! 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-06-24 20:14:10 UTC

Line 2062:1: E302 expected 2 blank lines, found 1 Line 2063:12: E225 missing whitespace around operator Line 2069:33: E231 missing whitespace after ',' Line 2069:57: E231 missing whitespace after ',' Line 2069:65: E226 missing whitespace around arithmetic operator

##[error]pandas/tests/groupby/test_groupby.py:2062: <- trailing whitespaces found ##[error]./pandas/tests/groupby/test_groupby.py:15:1:F811:redefinition of unused 'Series' from line 11 ##[error]ERROR: /home/runner/work/pandas/pandas/pandas/tests/groupby/test_groupby.py Imports are incorrectly sorted.

MarcoGorelli

Thanks @daxid !

MarcoGorelli · 2020-03-30T09:57:09Z

pandas/tests/groupby/test_groupby.py

@@ -2057,3 +2057,15 @@ def test_groups_repr_truncates(max_seq_items, expected):

        result = df.groupby(np.array(df.a)).groups.__repr__()
        assert result == expected
+
+
+def test_groupby_period_index():


Add the issue number here:

def test_groupby_period_index(): # GH 32108

MarcoGorelli · 2020-03-30T09:58:43Z

pandas/tests/groupby/test_groupby.py

+
+def test_groupby_period_index():
+    periods = 2
+    index = period_range(start='2018-01', periods=periods, freq='M')


Looks like you haven't run black, and perhaps not flake8 either. See code standards

dbeniamine · 2020-03-30T14:20:43Z

I've done the corrections, all tests are passing now, sorry for the delay

jreback · 2020-03-30T21:53:07Z

pandas/tests/groupby/test_groupby.py

@@ -2057,3 +2057,16 @@ def test_groups_repr_truncates(max_seq_items, expected):

        result = df.groupby(np.array(df.a)).groups.__repr__()
        assert result == expected
+
+


can you move this under test_grouper_index_types (same file). ping on green.

The function test_grouper_index_types is in the file pandas/tests/groupby/test_grouping.py our test is in pandas/tests/groupby/test_groupby.py, do you want us to move our test to test_grouping.py ?

yes pls move this

MarcoGorelli · 2020-04-04T11:05:46Z

pandas/tests/groupby/test_grouping.py

+        # GH 32108
+        periods = 2
+        index = pd.period_range(start="2018-01", periods=periods, freq="M")
+        periodSerie = pd.Series(range(periods), index=index)


naming this periodSerie doesn't conform to PEP8 - sorry for not bringing this up sooner, I thought flake8 would've caught it. seems there's an open issue to add extra plugins though (#20588)

Function and Variable Names
Function names should be lowercase, with words separated by underscores as necessary to improve readability.

Variable names follow the same convention as function names.

mixedCase is allowed only in contexts where that's already the prevailing style (e.g. threading.py), to retain backwards compatibility.

https://www.python.org/dev/peps/pep-0008/

Done, I hope it's all good now :)

simonjayhawkins · 2020-05-08T16:28:40Z

@daxid thanks for working on this. seems to have been lost in the shuffle. can you merge master to trigger ci.

dbeniamine · 2020-05-08T17:26:46Z

@daxid thanks for working on this. seems to have been lost in the shuffle. can you merge master to trigger ci.

done

jreback · 2020-06-14T17:19:45Z

can you merge master

dbeniamine · 2020-06-15T08:08:59Z

can you merge master

done

jreback · 2020-06-15T12:18:28Z

pandas/tests/groupby/test_grouping.py

+        period_serie.index.name = "Month"
+        result = period_serie.groupby(period_serie.index.month).sum()
+
+        expected = pd.Series(range(0, periods), index=range(1, periods + 1))


you can construct the Index(...., name=...)

jreback · 2020-06-15T12:18:48Z

pandas/tests/groupby/test_grouping.py

+    def test_grouper_period_index(self):
+        # GH 32108
+        periods = 2
+        index = pd.period_range(start="2018-01", periods=periods, freq="M")


pass name=

pandas/tests/groupby/test_grouping.py

jreback · 2020-06-15T12:20:37Z

pandas/tests/groupby/test_grouping.py

@@ -149,6 +149,18 @@ def test_grouper_index_types(self):
            df.index = list(reversed(df.index.tolist()))
            df.groupby(list("abcde")).apply(lambda x: x)

+    def test_grouper_period_index(self):
+        # GH 32108


move this to pandas/tests/groupby/test_timegrouper.py

dbeniamine · 2020-06-23T11:11:53Z

Done once again

simonjayhawkins

Thanks @daxid generally lgtm can you address #33105 (comment)

simonjayhawkins · 2020-06-23T13:42:39Z

pandas/tests/groupby/test_timegrouper.py

+
+        expected = pd.Series(
+            range(0, periods),
+            index=Index(range(1, periods + 1), name=period_serie.index.name),


maybe have name as index.name or just be explicit and use name="Month"

dbeniamine · 2020-06-23T16:01:58Z

Is that version good for you ?

simonjayhawkins · 2020-06-23T18:45:09Z

it appears that #33105 (comment) has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid

dbeniamine · 2020-06-24T17:14:15Z

it appears that #33105 (comment) has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid

I am not sure to understand you request, do you want us to add a name= parameter in the period_serie construction or do you want us to change the name period_serie to something else, maybe month_serie ?

MarcoGorelli · 2020-06-24T18:20:36Z

it appears that #33105 (comment) has not been addressed period_serie -> period_series otherwise lgtm. Thanks @daxid

I am not sure to understand you request, do you want us to add a name= parameter in the period_serie construction or do you want us to change the name period_serie to something else, maybe month_serie ?

Pretty sure the request is to rename period_serie to period_series

dbeniamine · 2020-06-24T20:16:06Z

Pretty sure the request is to rename period_serie to period_series

Thanks did now saw the s so I wasn't understing this request, should be good now

jreback · 2020-06-24T22:36:04Z

thanks @daxid

…od index) (pandas-dev#33105)

Add test for pandas-dev#32108

74e65a8

daxid added 2 commits March 28, 2020 21:32

MarcoGorelli suggested changes Mar 30, 2020

View reviewed changes

dbeniamine added 2 commits March 30, 2020 14:36

Fix linter issues

0cd603f

Black formating

35deb44

jreback added Groupby Period Period data type Testing pandas testing functions or related to the test suite labels Mar 30, 2020

jreback added this to the 1.1 milestone Mar 30, 2020

jreback requested changes Mar 30, 2020

View reviewed changes

Move test to test_grouping.py

e6f3884

MarcoGorelli reviewed Apr 4, 2020

View reviewed changes

Conform to PEP8

fd3c0db

Merge branch 'master' of https://github.com/pandas-dev/pandas

4dac36a

Merge branch 'master' of https://github.com/pandas-dev/pandas

7fbfb65

jreback requested changes Jun 15, 2020

View reviewed changes

dbeniamine force-pushed the master branch from a6a01cb to 364429d Compare June 23, 2020 10:34

New modifications for PR

4379fe3

dbeniamine force-pushed the master branch from 364429d to 4379fe3 Compare June 23, 2020 10:51

simonjayhawkins reviewed Jun 23, 2020

View reviewed changes

Change index name

ad9fac6

dbeniamine force-pushed the master branch from bffb8b7 to ad9fac6 Compare June 23, 2020 15:13

Rename period_series

408da64

jreback approved these changes Jun 24, 2020

View reviewed changes

jreback merged commit 3f4f564 into pandas-dev:master Jun 24, 2020

fangchenli pushed a commit to fangchenli/pandas that referenced this pull request Jun 27, 2020

Add test for pandas-dev#32108 (error with groupby on series with peri…

ecff72a

…od index) (pandas-dev#33105)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add test for #32108 (error with groupby on series with period index) #33105

Add test for #32108 (error with groupby on series with period index) #33105

daxid commented Mar 28, 2020

pep8speaks commented Mar 28, 2020 •

edited

Loading

MarcoGorelli left a comment

MarcoGorelli Mar 30, 2020

MarcoGorelli Mar 30, 2020

dbeniamine commented Mar 30, 2020

jreback Mar 30, 2020

dbeniamine Mar 31, 2020

jreback Mar 31, 2020

dbeniamine Mar 31, 2020

MarcoGorelli Apr 4, 2020 •

edited

Loading

dbeniamine Apr 4, 2020

simonjayhawkins commented May 8, 2020

dbeniamine commented May 8, 2020

jreback commented Jun 14, 2020

dbeniamine commented Jun 15, 2020

jreback Jun 15, 2020

jreback Jun 15, 2020

jreback Jun 15, 2020

dbeniamine commented Jun 23, 2020

simonjayhawkins left a comment

simonjayhawkins Jun 23, 2020

dbeniamine commented Jun 23, 2020

simonjayhawkins commented Jun 23, 2020

dbeniamine commented Jun 24, 2020

MarcoGorelli commented Jun 24, 2020

dbeniamine commented Jun 24, 2020

jreback commented Jun 24, 2020

		@@ -2057,3 +2057,16 @@ def test_groups_repr_truncates(max_seq_items, expected):

		result = df.groupby(np.array(df.a)).groups.__repr__()
		assert result == expected

Add test for #32108 (error with groupby on series with period index) #33105

Add test for #32108 (error with groupby on series with period index) #33105

Conversation

daxid commented Mar 28, 2020

pep8speaks commented Mar 28, 2020 • edited Loading

Comment last updated at 2020-06-24 20:14:10 UTC

MarcoGorelli left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbeniamine commented Mar 30, 2020

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

MarcoGorelli Apr 4, 2020 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

simonjayhawkins commented May 8, 2020

dbeniamine commented May 8, 2020

jreback commented Jun 14, 2020

dbeniamine commented Jun 15, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbeniamine commented Jun 23, 2020

simonjayhawkins left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

dbeniamine commented Jun 23, 2020

simonjayhawkins commented Jun 23, 2020

dbeniamine commented Jun 24, 2020

MarcoGorelli commented Jun 24, 2020

dbeniamine commented Jun 24, 2020

jreback commented Jun 24, 2020

pep8speaks commented Mar 28, 2020 •

edited

Loading

MarcoGorelli Apr 4, 2020 •

edited

Loading