BUG: Allow non-callable attributes in aggregate function. Fixes GH16405 #16458

pvomelveny · 2017-05-23T16:07:13Z

closes BUG: DataFrame.agg({'col': 'size'}) not working #16405
tests added / passed
passes git diff upstream/master --name-only -- '*.py' | flake8 --diff
whatsnew entry

TomAugspurger · 2017-05-23T17:38:14Z

Can you add the test from the original issue here? It can go in pandas/tests/frame/test_apply.py

Also could use a release note in doc/source/whatsnew/v0.20.2.txt, under the bugfix section.

codecov · 2017-05-23T18:40:00Z

Codecov Report

Merging #16458 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #16458      +/-   ##
==========================================
+ Coverage   90.43%   90.43%   +<.01%     
==========================================
  Files         161      161              
  Lines       51045    51049       +4     
==========================================
+ Hits        46161    46165       +4     
  Misses       4884     4884

Flag	Coverage Δ
#multiple	`88.27% <100%> (ø)`	⬆️
#single	`40.16% <0%> (-0.01%)`	⬇️

Impacted Files	Coverage Δ
pandas/core/base.py	`96.21% <100%> (+0.03%)`	⬆️

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 e81f3cc...0a367c6. Read the comment docs.

pvomelveny · 2017-05-23T21:16:32Z

I may have gone a bit overboard with the tests... although somehow I made coverage worse overall?

jreback · 2017-05-24T12:13:01Z

pandas/core/base.py

@@ -378,7 +378,7 @@ def aggregate(self, func, *args, **kwargs):
    def _try_aggregate_string_function(self, arg, *args, **kwargs):
        """
        if arg is a string, then try to operate on it:
-        - try to find a function on ourselves
+        - try to find an attribute on ourselves


function (or attribute)

jreback · 2017-05-24T12:13:19Z

doc/source/whatsnew/v0.20.2.txt

@@ -83,6 +83,7 @@ Reshaping
 - Bug in ``DataFrame.stack`` with unsorted levels in MultiIndex columns (:issue:`16323`)
 - Bug in ``pd.wide_to_long()`` where no error was raised when ``i`` was not a unique identifier (:issue:`16382`)
 - Bug in ``Series.isin(..)`` with a list of tuples (:issue:`16394`)
+- Bug in ``DataFrame.agg`` and ``Series.agg`` with aggregating on non-callable attributes (:issue:`16404`)


I think this only affects .size, so say that

As it works right now, you really can use any attribute in the aggregation, so something like:

In [20]: df Out[20]: A B C 0 NaN 1.0 foo 1 2.0 NaN None 2 3.0 3.0 bar In [21]: df.agg({'A': ['count', 'size', 'name'], ...: 'B': ['count', 'size', 'name', 'data'], ...: 'C': 'size'}) ...: Out[21]: A B C count 2 2 NaN data NaN [1.0, nan, 3.0] NaN name A B NaN size 3 3 3.0

works.

If it should only affect size, we could change the check in base.py to just see if the arg passed is 'size'.

This one's not actually outdated. I just messed up the bug # and had to fix it. The question of this only affecting size is still open

jreback · 2017-05-24T12:13:27Z

pandas/core/base.py

-            return f(*args, **kwargs)
+            if callable(f):
+                return f(*args, **kwargs)
+            # people may try to aggregate on a non-callable attribute


jreback · 2017-05-24T12:15:56Z

pandas/tests/frame/test_apply.py

+                                 'B': {'count': 2, 'size': 3},
+                                 'C': {'count': 2, 'size': 3}})
+
+        assert_frame_equal(result1.reindex_like(expected),


you can pass check_like=True to do this reindex comparisons.

jreback · 2017-05-24T12:16:57Z

pandas/tests/frame/test_apply.py

+                           result2.reindex_like(expected))
+        assert_frame_equal(result2.reindex_like(expected), expected)
+
+        # Just functional string arg is same as calling df.arg()


can you add a minimal tests to series/test_apply.py

jreback · 2017-05-24T12:18:22Z

pandas/tests/frame/test_apply.py

+        assert_series_equal(result, expected)
+
+        # Trying to to the same w/ non-function tries to pass args
+        # which we explicitly forbid


this actually should work, what are the args/kwargs that are actually there? (ithere maybe internal ones that need to be remove FYI, e.g. _level or something)

TomAugspurger · 2017-05-30T21:36:20Z

@jreback are you OK with this? I believe the discussion was whether we could have a simpler implementation by special-casing 'size'?

jreback · 2017-05-30T22:22:00Z

can't change size as it's already defined as a property (for numpy compat)

jreback

very small doc change. ping on green.

jreback · 2017-05-30T23:07:07Z

doc/source/whatsnew/v0.20.2.txt

@@ -89,6 +89,7 @@ Reshaping
 - Bug in ``pd.wide_to_long()`` where no error was raised when ``i`` was not a unique identifier (:issue:`16382`)
 - Bug in ``Series.isin(..)`` with a list of tuples (:issue:`16394`)
 - Bug in construction of a ``DataFrame`` with mixed dtypes including an all-NaT column. (:issue:`16395`)
+- Bug in ``DataFrame.agg`` and ``Series.agg`` with aggregating on non-callable attributes (:issue:`16405`)


DataFrame.agg() and Series.agg()

jreback · 2017-05-30T23:08:00Z

pandas/tests/frame/test_apply.py

@@ -635,3 +635,46 @@ def test_nuiscance_columns(self):
        expected = DataFrame([[6, 6., 'foobarbaz']],
                             index=['sum'], columns=['A', 'B', 'C'])
        assert_frame_equal(result, expected)
+
+    def test_non_callable_aggregates(self):
+


can you add a comment of .size here (and how its a property of Series/DataFrame and hence we allow this)

jreback · 2017-06-01T10:35:22Z

thanks!

…05 (pandas-dev#16458) (cherry picked from commit a67c7aa)

…05 (#16458) (cherry picked from commit a67c7aa)

…05 (pandas-dev#16458)

TomAugspurger added this to the 0.20.2 milestone May 23, 2017

TomAugspurger added Bug Reshaping Concat, Merge/Join, Stack/Unstack, Explode labels May 23, 2017

jreback requested changes May 24, 2017

View reviewed changes

pvomelveny added 2 commits May 25, 2017 10:38

BUG: Allow non-callable attributes in aggregate function. Fixes GH16405

dc17dec

Address changes requested in PR

a117022

jreback approved these changes May 30, 2017

View reviewed changes

minor doc corrections

0a367c6

jreback merged commit a67c7aa into pandas-dev:master Jun 1, 2017

jreback added the Needs Backport label Jun 1, 2017

TomAugspurger pushed a commit to TomAugspurger/pandas that referenced this pull request Jun 1, 2017

BUG: Allow non-callable attributes in aggregate function. Fixes GH164…

4373621

…05 (pandas-dev#16458) (cherry picked from commit a67c7aa)

TomAugspurger pushed a commit that referenced this pull request Jun 4, 2017

BUG: Allow non-callable attributes in aggregate function. Fixes GH164…

4008d90

…05 (#16458) (cherry picked from commit a67c7aa)

TomAugspurger removed the Needs Backport label Jun 4, 2017

Kiv pushed a commit to Kiv/pandas that referenced this pull request Jun 11, 2017

BUG: Allow non-callable attributes in aggregate function. Fixes GH164…

6d761b4

…05 (pandas-dev#16458)

stangirala pushed a commit to stangirala/pandas that referenced this pull request Jun 11, 2017

BUG: Allow non-callable attributes in aggregate function. Fixes GH164…

7ac663b

…05 (pandas-dev#16458)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: Allow non-callable attributes in aggregate function. Fixes GH16405 #16458

BUG: Allow non-callable attributes in aggregate function. Fixes GH16405 #16458

pvomelveny commented May 23, 2017 •

edited

Loading

TomAugspurger commented May 23, 2017

codecov bot commented May 23, 2017 •

edited

Loading

pvomelveny commented May 23, 2017 •

edited

Loading

jreback May 24, 2017

jreback May 24, 2017

pvomelveny May 24, 2017

pvomelveny May 25, 2017

jreback May 24, 2017

jreback May 24, 2017

jreback May 24, 2017

jreback May 24, 2017

TomAugspurger commented May 30, 2017

jreback commented May 30, 2017

jreback left a comment

jreback May 30, 2017

jreback May 30, 2017

jreback commented Jun 1, 2017

BUG: Allow non-callable attributes in aggregate function. Fixes GH16405 #16458

BUG: Allow non-callable attributes in aggregate function. Fixes GH16405 #16458

Conversation

pvomelveny commented May 23, 2017 • edited Loading

TomAugspurger commented May 23, 2017

codecov bot commented May 23, 2017 • edited Loading

Codecov Report

pvomelveny commented May 23, 2017 • edited Loading

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

TomAugspurger commented May 30, 2017

jreback commented May 30, 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

jreback commented Jun 1, 2017

pvomelveny commented May 23, 2017 •

edited

Loading

codecov bot commented May 23, 2017 •

edited

Loading

pvomelveny commented May 23, 2017 •

edited

Loading