BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 #21957

topper-123 · 2018-07-17T23:03:08Z

closes BUG: GroupBy.count arg minlength passed to np.bincount must be None rather than 0 for np<1.13 #21956
tests added / passed
passes git diff upstream/master -u -- "*.py" | flake8 --diff
whatsnew entry

See #21956 for details.

jreback · 2018-07-17T23:18:31Z

pandas/tests/groupby/test_counting.py

+    def test_count_with_only_nans_in_first_group(self):
+        # GH21956
+        df = DataFrame({'A': [np.nan, np.nan], 'B': ['a', 'b'], 'C': [1, 2]})
+        result = df.groupby(['A', 'B']).C.count()


can u do an assert_produces_warning()

here (should not show a warning)

This does not produce any warnings for me.

What should look for in travis? Grep "SeriesGroupBy.count"?

just look at the warnings in the 3.6 log

Ok, will look in the morning.

ok, can you assert that this produces no warning (e.g. it IS producing a warning now in numpy < 1.13)

jreback · 2018-07-17T23:18:48Z

doc/source/whatsnew/v0.24.0.txt

@@ -471,7 +471,7 @@ Groupby/Resample/Rolling

 - Bug in :func:`pandas.core.groupby.GroupBy.first` and :func:`pandas.core.groupby.GroupBy.last` with ``as_index=False`` leading to the loss of timezone information (:issue:`15884`)
 - Bug in :meth:`DatetimeIndex.resample` when downsampling across a DST boundary (:issue:`8531`)
-
+- Bug in :func:`pandas.core.groupby.GroupBy.count` when using numpy < 1.13 and ngroups=0 (:issue:`21956`).


a note is not necessary this is not user visible

The bug shown in #21956 is user visible...

jreback · 2018-07-17T23:19:02Z

pandas/core/groupby/generic.py

@@ -1207,7 +1208,10 @@ def count(self):

        mask = (ids != -1) & ~isna(val)
        ids = ensure_platform_int(ids)
-        out = np.bincount(ids[mask], minlength=ngroups or 0)
+        minlength = ngroups or 0


can u simplify this

Remove the " or 0" part?

I've made this a one-liner in the newest update.

codecov · 2018-07-17T23:31:17Z

Codecov Report

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

@@            Coverage Diff            @@
##             master   #21957   +/-   ##
=========================================
  Coverage          ?   92.02%           
=========================================
  Files             ?      170           
  Lines             ?    50710           
  Branches          ?        0           
=========================================
  Hits              ?    46664           
  Misses            ?     4046           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.42% <100%> (?)`
#single	`42.31% <0%> (?)`

Impacted Files	Coverage Δ
pandas/core/groupby/generic.py	`86.8% <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 495a0c1...32a5fcf. Read the comment docs.

jreback · 2018-07-18T10:39:05Z

pandas/core/groupby/generic.py

@@ -1207,7 +1208,8 @@ def count(self):

        mask = (ids != -1) & ~isna(val)
        ids = ensure_platform_int(ids)
-        out = np.bincount(ids[mask], minlength=ngroups or 0)
+        minlength = ngroups or (None if _np_version_under1p13 else 0)


what I meant is can you just pass None always

I think you are right, th doc string says "minlength : int, optional" and it accepts None for numpy >1.13. I'll change it.

jreback · 2018-07-18T10:39:36Z

doc/source/whatsnew/v0.23.4.txt

@@ -31,6 +31,7 @@ Bug Fixes
 **Groupby/Resample/Rolling**

 - Bug where calling :func:`DataFrameGroupBy.agg` with a list of functions including ``ohlc`` as the non-initial element would raise a ``ValueError`` (:issue:`21716`)
+- Bug in :func:`pandas.core.groupby.SeriesGroupBy.count` when using numpy < 1.13 and ngroups=0 (:issue:`21956`).


is there anything user visible atm?

Yes, as the per the example in #21956:

>>> df = pd.DataFrame({'A': [np.nan, np.nan], 'B': ['a', 'b'], 'C':[1,2]}) >>> df.groupby(['A', 'B']).C.count() ValueError: minlength must be positive # when numpy <1.13

jreback · 2018-07-25T12:20:45Z

doc/source/whatsnew/v0.23.4.txt

@@ -32,6 +32,7 @@ Bug Fixes

 - Bug where calling :func:`DataFrameGroupBy.agg` with a list of functions including ``ohlc`` as the non-initial element would raise a ``ValueError`` (:issue:`21716`)
 - Bug in ``roll_quantile`` caused a memory leak when calling ``.rolling(...).quantile(q)`` with ``q`` in (0,1) (:issue:`21965`)
+- Bug in :func:`pandas.core.groupby.SeriesGroupBy.count` when using numpy < 1.13 and ngroups=0 (:issue:`21956`).


again, is this actually a user facing change? (not against the note), just this is not understandable to user.

Ok, new explanation uploaded.

jreback · 2018-07-25T13:04:24Z

pandas/tests/groupby/test_counting.py

+    def test_count_with_only_nans_in_first_group(self):
+        # GH21956
+        df = DataFrame({'A': [np.nan, np.nan], 'B': ['a', 'b'], 'C': [1, 2]})
+        result = df.groupby(['A', 'B']).C.count()


ok, can you assert that this produces no warning (e.g. it IS producing a warning now in numpy < 1.13)

topper-123 · 2018-07-25T13:50:02Z

@jreback I got a straight up ValueError in master, so no warning. That ValueError is not in this PR, obviously

I've got a feeling I don't understand you. Could you give an example where a warning is emitted?

jreback · 2018-07-26T12:39:43Z

doc/source/whatsnew/v0.23.4.txt

@@ -32,6 +32,8 @@ Bug Fixes

 - Bug where calling :func:`DataFrameGroupBy.agg` with a list of functions including ``ohlc`` as the non-initial element would raise a ``ValueError`` (:issue:`21716`)


move this to 0.24.0, as this wont' backport cleanly.

pep8speaks · 2018-07-26T12:53:22Z

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

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

Comment last updated on July 26, 2018 at 12:58 Hours UTC

jreback

lgtm. ping on green.

topper-123 · 2018-07-26T20:59:03Z

Something's wrong with travis, else green:

Worker information
hostname: 1773d14a-144b-4458-b6fe-9060bb57620b@1.production-2-worker-org-09-packet
version: v4.0.0 https://github.com./travis-ci/worker/tree/e5cb567e10c0eefe380e81c9a2229ac8fb6a16ce
instance: 2a708b5 travisci/ci-garnet:packer-1512502276-986baf0 (via amqp)
startup: 2.24796375s
No output has been received in the last 10m0s, this potentially indicates a stalled build or something wrong with the build itself.
Check the details on how to adjust your build configuration on: https://docs.travis-ci.com/user/common-build-problems/#Build-times-out-because-no-output-was-received
The build has been terminated

jreback · 2018-07-28T13:51:05Z

thanks @topper-123

* master: BENCH: asv csv reading benchmarks no longer read StringIO objects off the end (pandas-dev#21807) BUG: df.agg, df.transform and df.apply use different methods when axis=1 than when axis=0 (pandas-dev#21224) BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 (pandas-dev#21957) CLN: Vbench to asv conversion script (pandas-dev#22089) consistent docstring (pandas-dev#22066) TST: skip pytables test with not-updated pytables conda package (pandas-dev#22099) CLN: Remove Legacy MultiIndex Index Compatibility (pandas-dev#21740) DOC: Reword doc for filepath_or_buffer in read_csv (pandas-dev#22058) BUG: rolling with MSVC 2017 build (pandas-dev#21813)

jreback · 2018-08-01T10:20:01Z

so these are the warnings: https://travis-ci.org/pandas-dev/pandas/jobs/410445444

/home/travis/build/pandas-dev/pandas/pandas/core/groupby/generic.py:1209: DeprecationWarning: 0 should be passed as minlength instead of None; this will error in future.
    out = np.bincount(ids[mask], minlength=ngroups or None)

can you PR to fix this. I guess we have to switch on the version and actually pass 0 rather than None for > 113

…ust be None for np<1.13 (pandas-dev#21957)

topper-123 changed the title ~~BUG: fix bug where np.bincount default arg minlength must be None for np<1.13~~ BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 Jul 17, 2018

jreback requested changes Jul 17, 2018

View reviewed changes

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch 2 times, most recently from 2d4ab97 to d65615b Compare July 17, 2018 23:20

gfyoung added Groupby Compat pandas objects compatability with Numpy or Python functions labels Jul 18, 2018

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from 91343df to 97ead75 Compare July 18, 2018 10:31

jreback requested changes Jul 25, 2018

View reviewed changes

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch 2 times, most recently from 2b202e0 to b250bce Compare July 25, 2018 10:40

jreback requested changes Jul 25, 2018

View reviewed changes

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from b250bce to eea3c76 Compare July 25, 2018 12:34

jreback requested changes Jul 25, 2018

View reviewed changes

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from eea3c76 to 4e59555 Compare July 25, 2018 18:36

fix bug where np.bincount default arg minlength must be None for np<1.13

2fe943a

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from 4e59555 to 5c978e2 Compare July 25, 2018 20:53

jreback requested changes Jul 26, 2018

View reviewed changes

jreback added this to the 0.24.0 milestone Jul 26, 2018

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from 5c978e2 to 49f30c2 Compare July 26, 2018 12:53

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from 49f30c2 to 17ad763 Compare July 26, 2018 12:54

changes according to comments

32a5fcf

topper-123 force-pushed the groupby_count_with_np_under_v1.13 branch from 17ad763 to 32a5fcf Compare July 26, 2018 12:58

jreback approved these changes Jul 26, 2018

View reviewed changes

jreback merged commit bb451e8 into pandas-dev:master Jul 28, 2018

topper-123 mentioned this pull request Aug 1, 2018

Fix deprecation warning in SeriesGroupBy.count #22155

Merged

topper-123 deleted the groupby_count_with_np_under_v1.13 branch August 1, 2018 10:43

Sup3rGeo pushed a commit to Sup3rGeo/pandas that referenced this pull request Oct 1, 2018

BUG: bug in GroupBy.count where arg minlength passed to np.bincount m…

10a95d5

…ust be None for np<1.13 (pandas-dev#21957)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 #21957

BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 #21957

topper-123 commented Jul 17, 2018

jreback Jul 17, 2018

topper-123 Jul 17, 2018

jreback Jul 17, 2018

topper-123 Jul 17, 2018

jreback Jul 25, 2018

jreback Jul 17, 2018

topper-123 Jul 17, 2018

jreback Jul 17, 2018

topper-123 Jul 17, 2018

topper-123 Jul 18, 2018

codecov bot commented Jul 17, 2018 •

edited

Loading

jreback Jul 18, 2018

topper-123 Jul 25, 2018

jreback Jul 18, 2018

topper-123 Jul 25, 2018

jreback Jul 25, 2018

topper-123 Jul 25, 2018

jreback Jul 25, 2018

topper-123 commented Jul 25, 2018

jreback Jul 26, 2018

pep8speaks commented Jul 26, 2018 •

edited

Loading

jreback left a comment

topper-123 commented Jul 26, 2018

jreback commented Jul 28, 2018

jreback commented Aug 1, 2018 •

edited

Loading

		@@ -32,6 +32,8 @@ Bug Fixes

		- Bug where calling :func:`DataFrameGroupBy.agg` with a list of functions including ``ohlc`` as the non-initial element would raise a ``ValueError`` (:issue:`21716`)

BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 #21957

BUG: bug in GroupBy.count where arg minlength passed to np.bincount must be None for np<1.13 #21957

Conversation

topper-123 commented Jul 17, 2018

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

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jul 17, 2018 • edited Loading

Codecov Report

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 commented Jul 25, 2018

Choose a reason for hiding this comment

pep8speaks commented Jul 26, 2018 • edited Loading

Comment last updated on July 26, 2018 at 12:58 Hours UTC

jreback left a comment

Choose a reason for hiding this comment

topper-123 commented Jul 26, 2018

jreback commented Jul 28, 2018

jreback commented Aug 1, 2018 • edited Loading

codecov bot commented Jul 17, 2018 •

edited

Loading

pep8speaks commented Jul 26, 2018 •

edited

Loading

jreback commented Aug 1, 2018 •

edited

Loading