Added repr string for Grouper and TimeGrouper #18203

topper-123 · 2017-11-09T22:29:21Z

closes #xxxx
tests added / passed
[x ] passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

This is a new implementation of #17727. This is much simpler than the previous attempts. The outputs are now:

>>> pd.Grouper(key='A')
Grouper(key='A')
>>> pd.Grouper(key='A', freq='2T')
TimeGrouper(key='A', freq=<2 * Minutes>, sort=True, closed='left', label='left', convention='e')

Tests still need to be written. I can do them when/if this is ok.

jreback · 2017-11-09T22:32:35Z

pandas/core/groupby.py

@@ -233,6 +233,8 @@ class Grouper(object):

    >>> df.groupby(Grouper(level='date', freq='60s', axis=1))
    """
+    _attributes = (('key', None), ('level', None), ('freq', None),


again you don't need to specify the default, just list the attributes. The repr should include all the values, not just the defaults. Further this means you are defining defaults in 2 places, here and the init, not a good idea.

Ok, changed. Also added test for TimeGrouper.__repr__. Not sure where a test for Grouper.__repr__ should go.

Should a note about this go in the 0.21.1 or 0.22 whatsnew.rst?

pep8speaks · 2017-11-09T22:49:20Z

Hello @topper-123! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on November 27, 2017 at 01:12 Hours UTC

topper-123 · 2017-11-09T23:59:48Z

After changes according to @jreback, the output is now:

>>> pd.Grouper(key='A')
Grouper(key='A', level=None, freq=None, axis=0, sort=False)
>>> pd.Grouper(key='A', freq='H')
TimeGrouper(key='A', level=None, freq=<Hour>, sort=True, closed='left', label='left', how='mean', nperiods=None, axis=0, fill_method=None, limit=None, loffset=None, kind=None, convention='e', base=0)

jreback · 2017-11-10T00:02:35Z

so what you can do is skip printing attributes that are None
makes it’s shorter w/o loss of generality

jreback · 2017-11-10T00:03:05Z

pandas/core/groupby.py

@@ -333,6 +334,15 @@ def _set_grouper(self, obj, sort=False):
    def groups(self):
        return self.grouper.groups

+    def __repr__(self):
+        attrs_list = []
+        for attr_name in self._attributes:


can be a list comprehension

jreback · 2017-11-10T00:03:44Z

pandas/core/resample.py

@@ -1025,6 +1025,9 @@ class TimeGrouper(Grouper):
    Use begin, end, nperiods to generate intervals that cannot be derived
    directly from the associated object
    """
+    _attributes = ('key', 'level', 'freq', 'sort', 'closed', 'label',
+                   'how', 'nperiods', 'axis', 'fill_method', 'limit',


only should have public attributes

iow you don’t need any special ones for TimeGrouper (which is publicly deprecated anyhow)

A TimeGrouper object is returned when calling Grouper with a freq arg, IOW it's only the top level location of TimeGrouper that is deprecated, right?

yes however you will never actually directly construct one
point being is the args are the same as Grouper (maybe a couple of extra which are prob not documented)

can u add the missing arguments in the doc string of Grouper? eg convention, base, loffset i think are only public ones

Ah of course. Changed.

Considering your last comment, it's actually possible to to:

>>> pd.Grouper(key='A', freq='H', convention='s') TimeGrouper(key='A', freq=<Hour>, axis=0, sort=True)

Note the output now misses convention='s', even though it's been set.

Shouldn't TimeGrouper actually have its own _attributes to account for this issue?

yeah it should take a copy of the super (Grouper) and add to it

Added _attributes to TimeGrouper.

I don't know what base, loffset and convention do, and can't quite figure it out from the code, so can't add explanations to the doc string, sorry. I added them, but without explanations. Do you have a proposal explanation for each?

jreback · 2017-11-10T00:26:13Z

pandas/tests/test_resample.py

+        expected = ("TimeGrouper(key='A', freq=<Hour>, sort=True, "
+                    "closed='left', label='left', how='mean', axis=0, "
+                    "convention='e', base=0)")
+        assert result == expected


a better test is to actually construct the object and compare to the original

I don't understand you here. Do you mean compare the repr of Grouper and TimeGrouper?

no, use the repr to actually construct a new object (that's the point of a repr), and compare to the original

I still don't understand. Should the expected be wrapped in a repr()?

jreback

see my other comments

jreback · 2017-11-10T00:57:57Z

doc/source/whatsnew/v0.21.1.txt

@@ -22,7 +22,7 @@ Other Enhancements
 ^^^^^^^^^^^^^^^^^^

 - :meth:`Timestamp.timestamp` is now available in Python 2.7. (:issue:`17329`)
-
+- :class:`Grouper` and :class:`TimeGrouper` now have a friendly repr output.


issue number

jreback · 2017-11-25T16:23:28Z

can you rebase

codecov · 2017-11-26T01:17:35Z

Codecov Report

❗ No coverage uploaded for pull request base (master@38f41e6). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #18203   +/-   ##
=========================================
  Coverage          ?   91.38%           
=========================================
  Files             ?      163           
  Lines             ?    50066           
  Branches          ?        0           
=========================================
  Hits              ?    45753           
  Misses            ?     4313           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`89.19% <100%> (?)`
#single	`40.36% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/groupby.py	`92.03% <100%> (ø)`
pandas/core/resample.py	`96.16% <100%> (ø)`

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 38f41e6...833883f. Read the comment docs.

codecov · 2017-11-26T01:17:44Z

Codecov Report

Merging #18203 into master will decrease coverage by 0.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #18203      +/-   ##
==========================================
- Coverage   91.35%   91.33%   -0.02%     
==========================================
  Files         163      163              
  Lines       49796    49797       +1     
==========================================
- Hits        45490    45482       -8     
- Misses       4306     4315       +9

Flag	Coverage Δ
#multiple	`89.13% <100%> (ø)`	⬆️
#single	`40.79% <100%> (-0.06%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/groupby.py	`92.03% <100%> (ø)`	⬆️
pandas/core/resample.py	`96.34% <100%> (ø)`	⬆️
pandas/io/gbq.py	`25% <0%> (-58.34%)`	⬇️
pandas/core/frame.py	`97.81% <0%> (-0.1%)`	⬇️

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 f745e52...bdc5ac2. Read the comment docs.

jreback · 2017-11-26T01:30:40Z

pandas/core/resample.py

@@ -1025,7 +1025,10 @@ class TimeGrouper(Grouper):
    Use begin, end, nperiods to generate intervals that cannot be derived
    directly from the associated object
    """
-
+    _attributes = Grouper._attributes + ('closed', 'label', 'how',
+                                         'nperiods', 'fill_method', 'limit',


can you leave out nperiods, also fill_method and limit which are deprecated.

you can actually completely remove nperiods as this is non-functional.

jreback

lgtm. some comments.

jreback · 2017-11-26T01:31:05Z

pandas/tests/test_resample.py

@@ -3416,3 +3416,10 @@ def test_aggregate_with_nat(self):

            # if NaT is included, 'var', 'std', 'mean', 'first','last'
            # and 'nth' doesn't work yet
+
+    def test_repr(self):
+        result = repr(TimeGrouper(key='A', freq='H'))


add the issue number

jreback · 2017-11-26T01:31:26Z

pandas/tests/test_resample.py

+    def test_repr(self):
+        result = repr(TimeGrouper(key='A', freq='H'))
+        expected = ("TimeGrouper(key='A', freq=<Hour>, axis=0, sort=True, "
+                    "closed='left', label='left', how='mean', "


can you add a similar test for Grouper (in test_groupby)

topper-123 · 2017-11-26T09:40:02Z

green.

jreback · 2017-11-26T15:12:59Z

linting issue. otherwise lgtm. note that you can do

make lint-diff to see linting in your patch, locally

jreback · 2017-11-26T22:56:16Z

pandas/core/resample.py

@@ -1025,6 +1025,10 @@ class TimeGrouper(Grouper):
    Use begin, end, nperiods to generate intervals that cannot be derived
    directly from the associated object
    """
+    _attributes = Grouper._attributes + ('closed', 'label', 'how',
+                                         'nperiods', 'fill_method', 'limit',


remove nperiods, fill_method, and limit

and remove nperiods from the signature entirely, its is not used

Ok, but what's the benefit of not having fill_method and limit in _attributes?

because they are deprecated (see the maybe_process_deprecations)

Also, there's this

Notes ----- Use begin, end, nperiods to generate intervals that cannot be derived directly from the associated object

None of begin or end seem to exist, and if nperiods isn't used, I've just removed that note

yeah remove that, looks bogus

topper-123 · 2017-11-27T08:16:27Z

All green, @jreback

jreback · 2017-11-27T11:28:17Z

thanks!

(cherry picked from commit f7c79be)

jreback mentioned this pull request Nov 9, 2017

repr string for pd.Grouper #17727

Closed

2 tasks

jreback requested changes Nov 9, 2017

View reviewed changes

jreback added Groupby Output-Formatting __repr__ of pandas objects, to_string Resample resample method labels Nov 9, 2017

topper-123 force-pushed the Grouper_repr_II branch from eb6bc4c to deab64f Compare November 9, 2017 22:49

topper-123 force-pushed the Grouper_repr_II branch 3 times, most recently from a603bee to df67063 Compare November 9, 2017 23:16

jreback reviewed Nov 10, 2017

View reviewed changes

topper-123 force-pushed the Grouper_repr_II branch 2 times, most recently from 3add483 to 74aa59d Compare November 10, 2017 00:23

jreback reviewed Nov 10, 2017

View reviewed changes

topper-123 force-pushed the Grouper_repr_II branch from 74aa59d to bf6eb67 Compare November 10, 2017 00:27

jreback requested changes Nov 10, 2017

View reviewed changes

topper-123 force-pushed the Grouper_repr_II branch from bf6eb67 to e4b1d5e Compare November 10, 2017 01:18

topper-123 force-pushed the Grouper_repr_II branch from e4b1d5e to 833883f Compare November 26, 2017 01:17

jreback reviewed Nov 26, 2017

View reviewed changes

jreback requested changes Nov 26, 2017

View reviewed changes

topper-123 force-pushed the Grouper_repr_II branch from 833883f to 1a4747b Compare November 26, 2017 01:48

jreback modified the milestone: 0.22.0 Nov 26, 2017

jreback added this to the 0.21.1 milestone Nov 26, 2017

jreback added the Needs Backport label Nov 26, 2017

topper-123 force-pushed the Grouper_repr_II branch 2 times, most recently from 512c8e2 to 1d4e3d9 Compare November 26, 2017 20:16

jreback approved these changes Nov 26, 2017

View reviewed changes

jreback requested changes Nov 26, 2017

View reviewed changes

Added repr string for Grouper and TimeGrouper

bdc5ac2

topper-123 force-pushed the Grouper_repr_II branch from 1d4e3d9 to bdc5ac2 Compare November 27, 2017 01:12

jreback approved these changes Nov 27, 2017

View reviewed changes

jreback merged commit f7c79be into pandas-dev:master Nov 27, 2017

topper-123 deleted the Grouper_repr_II branch November 27, 2017 13:52

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Dec 8, 2017

Added repr string for Grouper and TimeGrouper (pandas-dev#18203)

92dc7a0

(cherry picked from commit f7c79be)

TomAugspurger pushed a commit that referenced this pull request Dec 11, 2017

Added repr string for Grouper and TimeGrouper (#18203)

8f09619

(cherry picked from commit f7c79be)

TomAugspurger removed the Needs Backport label Dec 11, 2017

Added repr string for Grouper and TimeGrouper #18203

Added repr string for Grouper and TimeGrouper #18203

Conversation

topper-123 commented Nov 9, 2017

Choose a reason for hiding this comment

topper-123 Nov 9, 2017 • edited Loading

Choose a reason for hiding this comment

pep8speaks commented Nov 9, 2017 • edited Loading

Comment last updated on November 27, 2017 at 01:12 Hours UTC

topper-123 commented Nov 9, 2017 • edited Loading

jreback commented Nov 10, 2017

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

topper-123 Nov 10, 2017 • edited Loading

Choose a reason for hiding this comment

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

jreback commented Nov 25, 2017

codecov bot commented Nov 26, 2017

Codecov Report

codecov bot commented Nov 26, 2017 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

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

topper-123 commented Nov 26, 2017

jreback commented Nov 26, 2017

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

topper-123 commented Nov 27, 2017

jreback commented Nov 27, 2017

topper-123 Nov 9, 2017 •

edited

Loading

pep8speaks commented Nov 9, 2017 •

edited

Loading

topper-123 commented Nov 9, 2017 •

edited

Loading

topper-123 Nov 10, 2017 •

edited

Loading

topper-123 Nov 10, 2017 •

edited

Loading

topper-123 Nov 10, 2017 •

edited

Loading

topper-123 Nov 10, 2017 •

edited

Loading

codecov bot commented Nov 26, 2017 •

edited

Loading