Breaking changes for sum / prod of empty / all-NA
State: closed

Changes the defaults for `min_count` so that `sum([])` and `sum([np.nan])` are 0 by default, and NaN with `min_count>=1`.

I'd recommend looking at only the latest commit until https://github.com/pandas-dev/pandas/pull/18876 is merged. I'll probably force push changes here to keep all the relevant changes in the last commit until https://github.com/pandas-dev/pandas/pull/18876 is in, rebase on that, and then start pushing changes regularly.

cc @jreback @jorisvandenbossche @shoyer @wesm 

Labels:
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations

Comments:
This turned up a slight inconsistency between:

a) `groupby([pd.Grouper(key1, freq), key2]).agg`
b) `groupby(key2).resample(freq).agg`

The first doesn't upsample to include "unobserved" values, while the latter does. An example:

```python
        df = pd.DataFrame({
            'A': [1] * 6 + [2] * 6,
            'B': pd.to_datetime(['2017'] * 3 +
                                ['2019'] * 3 +
                                ['2017'] * 3 +
                                ['2019'] * 3),
            'C': [1] * 12
        })
        a = df.groupby([pd.Grouper(key='B', freq='AS'), 'A']).C.count()
        b = df.set_index("B").sort_index().groupby("A").resample("AS").C.count()

```

```python

In [53]: a  # groupby([grouper, key]).agg
Out[53]:
B           A
2017-01-01  1    3
            2    3
2019-01-01  1    3
            2    3
Name: C, dtype: int64

In [54]: b  # groupby(key).resample().agg
Out[54]:
A  B
1  2017-01-01    3
   2018-01-01    0
   2019-01-01    3
2  2017-01-01    3
   2018-01-01    0
   2019-01-01    3
Name: C, dtype: int64
```

I've use `count` here, but the same thing is affecting sum and prod. Do we want these to be the same?
Hello @TomAugspurger! Thanks for updating the PR.



































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

##### Comment last updated on December 29, 2017 at 12:25 Hours UTC
RE: Grouoer/resample: that does look inconsistent. I think the resample behavior should be used for both?
Somehow I could give an explanation for the difference in behaviour between the groupby and resample example (resample: conform to new index given the frequency -> full time series, groupby: makes groups of my data based on given frequency -> only those groups contained in data). But of course there are also good reasons to have this consistent. 
But therefore: does this need to be fixed here? (is keeping it as it is complicating the PR?) If not, I would open a separate issue for it.
> But therefore: does this need to be fixed here? (is keeping it as it is complicating the PR?) If not, I would open a separate issue for it.

I'm not sure. We have two tests that relied on that equivalence to construct the expected result.
The only failing tests are the equivalence tests between `df.groupby(key).resample` and `df.groupby([key, Grouper])`.

> Somehow I could give an explanation for the difference in behaviour between the groupby and resample example

@jorisvandenbossche could you explain a bit further? I don't quite understand. IMO, saying `Grouper(key, freq='AS')` necessarily implies a resample.
Returning to https://github.com/pandas-dev/pandas/pull/18921#issuecomment-353746002, it's only an issue for when there are multiple groupers:

```python
In [14]: df = pd.DataFrame({"A": pd.to_datetime(['2015', '2017']), "B": [1, 1]})

In [15]: df.groupby(pd.Grouper(key="A", freq="AS")).B.count()
Out[15]:
A
2015-01-01    1
2016-01-01    0
2017-01-01    1
Freq: AS-JAN, Name: B, dtype: int64

In [16]: df.groupby([pd.Grouper(key="A", freq="AS"), [0, 0]]).B.count()
Out[16]:
A
2015-01-01  0    1
2017-01-01  0    1
Name: B, dtype: int64
```

Still trying to track down the appropriate spot for the fix. Will we want to include this bugfix in 0.22, or save it for 0.23.0?
Fixing this is turning into quite the rabbit hole :/
IIUC, the issue is in how the new groups are determined


```python
In [9]: df = pd.DataFrame({"A": pd.to_datetime(['2015', '2017']), "B": [1, 1]})

In [10]: gr1 = df.groupby(pd.Grouper(key="A", freq="AS")).B

In [11]: gr2 = df.groupby([pd.Grouper(key="A", freq="AS"), [0, 0]])

In [12]: gr1.grouper.result_index
Out[12]: DatetimeIndex(['2015-01-01', '2016-01-01', '2017-01-01'], dtype='datetime64[ns]', name='A', freq='AS-JAN')

In [13]: gr2.grouper.result_index
Out[13]:
MultiIndex(levels=[[2015-01-01 00:00:00, 2016-01-01 00:00:00, 2017-01-01 00:00:00], [0]],
           labels=[[0, 2], [0, 0]],
           names=['A', None])
```

So `gr2.grouper.result_index` has a level with no observations, the `2016-01-01`. Then when we go to construct the groups it's dropped:

```python
In [21]: gr2.grouper.groups
Out[21]:
{(Timestamp('2015-01-01 00:00:00'), 0): Int64Index([0], dtype='int64'),
 (Timestamp('2017-01-01 00:00:00'), 0): Int64Index([1], dtype='int64')}
```

I don't know if that's the root problem yet, or just a symptom.
I think we should split a fix for https://github.com/pandas-dev/pandas/pull/18921#issuecomment-354179920 off to 0.23.0. Fixing that inconsistency will mean other, larger changes which defeats the idea that "0.22.0 is 0.21.x + this NA changes."

I opened https://github.com/pandas-dev/pandas/issues/18971 for it.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=h1) Report
> Merging [#18921](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/dbec3c92e08063d247e0d28937c8695bbd66fe94?src=pr&el=desc) will **decrease** coverage by `0.04%`.
> The diff coverage is `93.33%`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/18921/graphs/tree.svg?width=650&src=pr&token=eZ4WkYLtcO&height=150)](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #18921      +/-   ##
==========================================
- Coverage    91.6%   91.56%   -0.05%     
==========================================
  Files         150      150              
  Lines       48939    48977      +38     
==========================================
+ Hits        44833    44845      +12     
- Misses       4106     4132      +26
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.92% <93.33%> (-0.05%)` | :arrow_down: |
| #single | `41.72% <40%> (+0.54%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `95.95% <ø> (ø)` | :arrow_up: |
| [pandas/core/groupby.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ3JvdXBieS5weQ==) | `92.14% <100%> (ø)` | :arrow_up: |
| [pandas/core/resample.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzYW1wbGUucHk=) | `96.39% <100%> (ø)` | :arrow_up: |
| [pandas/core/nanops.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbmFub3BzLnB5) | `96.68% <100%> (ø)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `84.95% <90%> (+0.05%)` | :arrow_up: |
| [pandas/plotting/\_converter.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb252ZXJ0ZXIucHk=) | `65.22% <0%> (-1.74%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `92.61% <0%> (-1.21%)` | :arrow_down: |
| [pandas/core/sparse/array.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3BhcnNlL2FycmF5LnB5) | `91.82% <0%> (-0.47%)` | :arrow_down: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `88.42% <0%> (-0.18%)` | :arrow_down: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `97.04% <0%> (-0.16%)` | :arrow_down: |
| ... and [4 more](https://codecov.io/gh/pandas-dev/pandas/pull/18921/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=footer). Last update [dbec3c9...a97e133](https://codecov.io/gh/pandas-dev/pandas/pull/18921?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

tiny comment. ok merging on green.
OK, all green.

I'm going to

1. go through this diff one more time
2. Merge it to master
3. Push a 0.23.0.dev0 tag to master
4. branch 0.22.x off 0.21.x
5. backport these two PRs to 0.22.x
All green. Here we go.
I not sure  understand the rationale for making ``Series([np.nan]).prod()`` equal 1. e.g. for percent-change calculations, a calculation for all ``nan`` series, should logically give ``nan``, IMO, but with this change it will give 0.

```python
>>> s = pd.Series([0, 0.05, 0.12])
>>> (1+s).prod() -1
0.17600000000000016  # ok
>>> s = pd.Series([0, 0.0, 0.0])
>>> (1+s).prod() -1
0.0  # ok
>>> s = pd.Series([np.nan, np.nan, np.nan])
>>> (1+s).prod() -1
0.0  # how do you interpret this?.
```

IMO percent calculation will be difficult to interpret, when product of ``nan``'s will equal 1, as no-change and not-defined will give back the same result of 0.0.

Is there a write-up on this, in case I'm misunderstanding this?
> I not sure understand the rationale for making Series([np.nan]).prod() equal 1.

The rationale is that this is consistent with how sum() works: 0 is the additive identity, and 1 is the multiplicative identity. It would be confusing to use a different rule.

There are absolutely use cases where these definitions for sum/prod is not desired, but it's impossible for the default behavior to work in all cases. Your use case of calculating percent change sounds like a good use for `min_count=1` or even `skipna=False`.