apply_frame_axis0 applies function to first group twice #21943

javadnoorb · 2018-07-17T04:01:19Z

closes df.groupby(.).plot.scatter() creates a spurious initial plot #21609
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

… and hexbin

removing PATH_MODE debugging flag

remove whitespaces

…s not exclude x-axis label and tick-labels

remove whitespace

changing for PEP8

fixing lint E231

… x,y (pandas-dev#20000) * Add support for list-like y argument * update whatsnew * add doc change for y * Add test cases and fix position args * don't copy save cols ahead of time and update whatsnew * address fdbck

…s-dev#20394)

* DOC: Improve the docstring of DataFrame.transpose() Co-authored-by: Carlos Eduardo Moreira dos Santos <cems@cemshost.com.br>

…0357)

* REF: Mock all S3 Tests Closes pandas-dev#19825

* TST: Fixed version comparison This failed to skip for 3.5.x because the micro component made it False. * Use pandas.compat * More pandas compat

javadnoorb · 2018-07-25T20:16:45Z

pandas/tests/plotting/test_groupby.py

+
+                def f(self):
+                    getattr(self.plot, name)(*args, **kwargs)
+                    # I have no idea why this is needed twice


RESOLVED: Function was returning next(counter_obj) when I wrote it.

TomAugspurger · 2018-07-26T13:14:43Z

@javadnoorb looks like there are unrelated commits on this PR now, do you need help with git stuff? https://pandas.pydata.org/pandas-docs/stable/contributing.html#working-with-the-code might be helpful as well.

TomAugspurger · 2018-07-26T13:32:34Z

pandas/tests/plotting/test_groupby.py

+     ])
+@td.skip_if_no_mpl
+def test_no_double_plot_for_first_group(plotting_method):
+    class extGroupByPlot(pd.core.groupby.groupby.GroupByPlot):


I think this test can be simplified.

How about

df = pd.DataFrame({'cat': [1, 1, 2, 2], 'x': [1, 3, 5, 7], 'y': [2, 4, 6, 8]}) df.groupby('cat').plot.scatter(x='x', y='y') assert plt.get_fignums() == [1, 2]

or something like that.

You'd need to replace .plot.scatter with your getattr(gr, plotting_method)(x='x', y='y'), but the basic idea is to test the output, the number of produced figures, is the number of groups.

That looks much better. I didn't know about get_fignums.

Do you by any chance know how to reset this? It works for the first plot method, but then keep accumulating as pytest.mark.parametrize is looped through.

Run plt.close() twice?

Each test should close all its state. plt.close('all') should do it.

javadnoorb · 2018-07-26T13:35:57Z

@TomAugspurger yeah. I messed up while syncing my fork. There were too many conflicts after the merge and I manually removed mine against the master, and some newlines were left here and there. Was I supposed to do this manually?

I think there was a delay in sync that caused this. Sorry about that. I'll clean it up.

javadnoorb · 2018-07-30T17:11:18Z

@jreback @toobaz @TomAugspurger. Just leaving a note. This should be ready for review. -thanks

TomAugspurger

It looks like there are two uses of pandas.core.groupby.base.plotting_methods.

core/groupby/ops.py, which you've updated
core/rgoupby/groupby.py in _make_wrapper, which hasn't been updated.

Can you try just adding these new methods to plotting_methods and seeing if anything breaks? Ideally we would just have the one frozenset. I initially added extra_plotting_methods just to test things out.

TomAugspurger · 2018-07-30T17:15:04Z

doc/source/whatsnew/v0.24.0.txt

- Bug in :func:'DataFrame.plot.scatter' and :func:'DataFrame.plot.hexbin' caused x-axis label and ticklabels to disappear when colorbar was on in IPython inline backend (:issue:`10611`, :issue:`10678`, and :issue:`20455`)
-
+- Bug in :func:`DataFrame.plot.scatter` and :func:`DataFrame.plot.hexbin` caused x-axis label and ticklabels to disappear when colorbar was on in IPython inline backend (:issue:`10611`, :issue:`10678`, and :issue:`20455`)
+- Bug in :func:`DataFrameGroupBy.plot.scatter()` resulted in the first group being plotted twice (:issue:`21609`)


I don't think DataFrameGroupBy.plot.scatter is included in our API docs http://pandas-docs.github.io/pandas-docs-travis/api.html. Probably OK to just do

``DataFrameGroupBy.plot.scatter``

instead of the :func:

TomAugspurger · 2018-07-30T17:16:32Z

pandas/core/groupby/groupby.py

@@ -311,6 +311,7 @@ def __getattr__(self, name):
        def attr(*args, **kwargs):
            def f(self):
                return getattr(self.plot, name)(*args, **kwargs)
+            f.__name__ = name


A comment explaining why we set the name here would be helpful.

javadnoorb · 2018-07-30T19:25:50Z

@TomAugspurger, I don't think plotting_methods and extra_plotting_methods should be merged. It leads to some errors like:

AttributeError: type object 'Series' has no attribute 'hexbin'

plotting_methods affects:

pandas/pandas/core/groupby/base.py

Line 80 in 26b3e7d

series_apply_whitelist = ((common_apply_whitelist |

and

pandas/pandas/core/groupby/base.py

Line 86 in 26b3e7d

dataframe_apply_whitelist = ((common_apply_whitelist |

which are then used as methods for dataframes and series.

I think only functions used in this doc can be methods of groupby.plot:

http://pandas.pydata.org/pandas-docs/version/0.16.2/generated/pandas.core.groupby.DataFrameGroupBy.plot.html

hist is the only exception because it has dual functionality (both for ~~series/dataframes~~ groupby and groupby.plot). But I think it should be sufficient to have it in plotting_methods.

Following the other point, adding extra_plotting_methods to core/goupby/groupby.py:_make_wrapper doesn't seem to do anything when I try it. These curried_with_axis functions are pretty old and have no issue number. I can't track down why they were written, but seems like they just explicitly add an axis to the plot if none is provided.

TomAugspurger · 2018-07-31T00:30:58Z

Thanks for the clarification on merging those two sets. Is the make_wrapper code dead? Does removing it break any tests? I’ve never touched them.

…

________________________________ From: Javad Noorbakhsh <notifications@github.com> Sent: Monday, July 30, 2018 2:25:56 PM To: pandas-dev/pandas Cc: Tom Augspurger; Mention Subject: Re: [pandas-dev/pandas] apply_frame_axis0 applies function to first group twice (#21943) @TomAugspurger<https://github.com/TomAugspurger>, I don't think plotting_methods and extra_plotting_methods should be merged. It leads to some errors like: AttributeError: type object 'Series' has no attribute 'hexbin' plotting_methods affects: https://github.com/pandas-dev/pandas/blob/26b3e7de56fdebaf1842b01d747a1a30a126c54a/pandas/core/groupby/base.py#L80 and https://github.com/pandas-dev/pandas/blob/26b3e7de56fdebaf1842b01d747a1a30a126c54a/pandas/core/groupby/base.py#L86 which are then used as methods for dataframes and series. I think only functions used in this doc can be methods of groupby.plot: http://pandas.pydata.org/pandas-docs/version/0.16.2/generated/pandas.core.groupby.DataFrameGroupBy.plot.html hist is the only exception because it has dual functionality (both for series/dataframes and groups). But I think it should be sufficient to have it in plotting_methods. Following the other point, adding extra_plotting_methods to core/goupby/groupby.py:_make_wrapper doesn't seem to do anything when I try it. These curried_with_axis functions are pretty old and have no issue number. I can't track down why they were written, but seems like they just explicitly add an axis to the plot if none is provided. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#21943 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIlKw6qDVSnqrNnc8dHZOliHPC3Cyks5uL13EgaJpZM4VSJaX>.

javadnoorb · 2018-07-31T03:25:51Z

No, it's not dead code. Just that it doesn't run when groupby.plot.apply is called. Commenting it out causes some tests to fail:

___________________________________________________________________________________________________ TestDataFrameGroupByPlots.test_series_groupby_plotting_nominally_works ___________________________________________________________________________________________________

self = <pandas.tests.plotting.test_groupby.TestDataFrameGroupByPlots object at 0x1a1fb72278>

    def test_series_groupby_plotting_nominally_works(self):
        n = 10
        weight = Series(np.random.normal(166, 20, size=n))
        height = Series(np.random.normal(60, 10, size=n))
        with tm.RNGContext(42):
            gender = np.random.choice(['male', 'female'], size=n)
    
        weight.groupby(gender).plot()
        tm.close()
>       height.groupby(gender).hist()

pandas/tests/plotting/test_groupby.py:49: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <pandas.core.groupby.generic.SeriesGroupBy object at 0x1a1fc10d30>, attr = 'hist'

    def __getattr__(self, attr):
        if attr in self._internal_names_set:
            return object.__getattribute__(self, attr)
        if attr in self.obj:
            return self[attr]
        # if hasattr(self.obj, attr):
        #     return self._make_wrapper(attr)
    
        raise AttributeError("%r object has no attribute %r" %
>                            (type(self).__name__, attr))
E       AttributeError: 'SeriesGroupBy' object has no attribute 'hist'

pandas/core/groupby/groupby.py:537: AttributeError

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

jreback · 2018-11-04T16:11:41Z

closing as stale. if you'd like to continue working, pls ping.

javadnoorb and others added 30 commits March 21, 2018 20:53

removed colorbars from _handle_shared_axes when called by scatterplot…

e2abff6

… and hexbin

removed colorbars from _handle_shared_axes when called by scatterplot…

0ea226f

… and hexbin

added a debug global variable

80f949f

removed whitespaces

8f52eab

Update _core.py

215fb89

removing PATH_MODE debugging flag

Update _core.py

7c6efe2

remove whitespaces

Update _tools.py

8c71f60

included tests for scatterplot and hexbin plot to ensure colorbar doe…

63b9ee6

…s not exclude x-axis label and tick-labels

Update test_frame.py

f401672

remove whitespace

Update test_frame.py

b821474

changing for PEP8

Update _tools.py

52c1fa8

changing for PEP8

fixing assert message

edf008d

Update test_frame.py

db6cf67

fixing lint E231

fixing style issues

186a09c

DOC: Improve the docstrings of CategoricalIndex.map (pandas-dev#20286)

e6980f3

DOC: update the DataFrame.to_hdf() docstirng (pandas-dev#20186)

cdc0240

DOC: enable docstring on DataFrame.columns/index (pandas-dev#20385)

4848dea

DOC: general docstring formatting fixes (pandas-dev#20449)

5aed4b5

parameterize tests in scalar/timedelta (pandas-dev#20428)

e2053c9

Removed dead groupby code (pandas-dev#20457)

3a76199

EHN: allow zip compression in to_pickle, to_json, to_csv (panda…

189dd8e

…s-dev#20394)

ENH: Sorting of ExtensionArrays (pandas-dev#19957)

c302b04

DOC: Improve the docstring of DataFrame.transpose() (pandas-dev#20254)

faefc89

* DOC: Improve the docstring of DataFrame.transpose() Co-authored-by: Carlos Eduardo Moreira dos Santos <cems@cemshost.com.br>

DOC: Update pandas.Series.copy docstring (pandas-dev#20261)

4a77c96

DOC: update the pandas.DataFrame.plot.box docstring (pandas-dev#20373)

e03b4b8

DOC: make deprecation warning more visible with red box (pandas-dev#2…

5ffb078

…0357)

TST: clean deprecation warnings & some parametrizing (pandas-dev#20467)

689b3fe

REF: Mock all S3 Tests (pandas-dev#20409)

7f97c13

* REF: Mock all S3 Tests Closes pandas-dev#19825

TST: Fixed version comparison (pandas-dev#20469)

9dd5111

* TST: Fixed version comparison This failed to skip for 3.5.x because the micro component made it False. * Use pandas.compat * More pandas compat

javadnoorb added 4 commits July 24, 2018 22:40

TST:skip_if_no_scipy

fa17da1

TST:skip_if_no_scipy

201d31f

TST:skip_if_no_scipy for density

6e69fcb

CLN/TST:lint

3170b89

javadnoorb commented Jul 25, 2018

View reviewed changes

javadnoorb added 2 commits July 25, 2018 22:57

TST: removed extra count

702943e

updating

6f007ae

Merge branch 'master' into fix_apply_frame_axis0

d5c909c

TomAugspurger reviewed Jul 26, 2018

View reviewed changes

javadnoorb added 4 commits July 26, 2018 14:20

TST/CLN: simplify test/clean git-merge mess

0c02dd7

TST: xref added

5a84301

TST: comment issue

1475d0b

TST: adding close all before (in case)

e934df4

javadnoorb mentioned this pull request Jul 26, 2018

BUG: groupby.plot.kde applies to index as well #22069

Open

timedout - repush

9480e83

TomAugspurger reviewed Jul 30, 2018

View reviewed changes

fixed comment

d15e9d4

javadnoorb added 5 commits July 30, 2018 23:27

CLN: adding comments to test/cleaning whatsnew entry

160f869

CLN: repush

6db7549

syncing

4e42be5

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

CLN: adding comments

7eb89e1

empty commit: failed tests seem unrelated

1058066

jreback closed this Nov 4, 2018

Uh oh!

apply_frame_axis0 applies function to first group twice #21943

apply_frame_axis0 applies function to first group twice #21943

Uh oh!

Conversation

javadnoorb commented Jul 17, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

TomAugspurger commented Jul 26, 2018

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javadnoorb commented Jul 26, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

javadnoorb commented Jul 30, 2018

Uh oh!

TomAugspurger left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javadnoorb commented Jul 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TomAugspurger commented Jul 31, 2018 via email

Uh oh!

javadnoorb commented Jul 31, 2018

Uh oh!

jreback commented Nov 4, 2018

Uh oh!

Uh oh!

javadnoorb commented Jul 17, 2018 •

edited

Loading

javadnoorb commented Jul 26, 2018 •

edited

Loading

javadnoorb commented Jul 30, 2018 •

edited

Loading