[WIP, ENH] Adds cumulative methods to ea
State: closed

- [x] closes #28385
- [x] tests added / passed
- [x] passes `black pandas`
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Problem specific

- [X] Add abstract tests similar to pandas/tests/extension/base/reduce.py
- [X] Add TestFixtures for Accumulations
- [x] Implement Accumulation for IntegerArray
- [ ] Implement Accumulation for DecimalArray - out of scope
- [ ] Implement Accumulation for DatetileLikeArrayMixin ? - out of scope
- [x] Use BaseNoAccumulateTest where applicable - implemented this for categorical

Labels:
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

Comments:
Sorry, I did not intend to have a proper code review, essentially, I just replaced reduce with accumulate and created duplicates. Will have a deeper dive into the consequences soon.

I'll follow your guidance and start implementing the test soon.

As you pointed out, we should often see a different Type `ExtensionArray[S]`. Additionally to your example, `cummax` could potentially return signed integers. E.g. on an array `[1, -1, 1, -1, ... ]`.
I was expecting that in extension arrays, the `_reduce` function is used to define e.g. `sum`, `max` 
etc. However, this does not seem to happen (maybe as of now). So I'm trying to find, where an ExtensionArray gets these functions. And I'm only guessing:

I'd assume that an extension array typically is seen as a Series. Here the `_reduce` function is defined. Moreover, we inherit from `base.IndexOpsMixin` which defines e.g. `max`, `min` etc.

On the other hand, on the Series, we perform e.g. `_add_numeric_operations` (defined for NDFrames (which inherit from pandas objects) which uses the `_reduce` function of the object.

So what is the right interpretation here? Thanks in advance




> I was expecting that in extension arrays, the _reduce function is used to define e.g. sum, max
etc. However, this does not seem to happen (maybe as of now). So I'm trying to find, where an ExtensionArray gets these functions.

This is up to EA authors how to implement.  e.g. `sum` is not necessarily well-defined for some EAs (like DatetimeArray) so we can't define it in the general case
Thanks for getting back to me. I just realized, that e.g. IntegerArray only uses `_reduce` and does not provide a `sum` property (which I expected). I hope to have a bit more time soon.
@datajanko - is this still active? Mind addressing the comments above and fix up the tests. Feel free to post on here if you run into any issues! 
Yes, I'm still working on this. I think so far I managed to create the abstract test classes. Next step is to implement the functionality for integer arrays. Which comments do you mean in particular? Maybe it might even make sense to split this issue into multiple. 
@TomAugspurger I just realized that your comment on the dtypes is crucial. Doing the cumsum on the first 100 digits, will directly raise this issue for Int8 dtypes. So what should be the strategy here?

In terms of cumsum, an implementation could be:

replace the data at masked positions with 0 (neutral element of addition). Compute the cumsum (using numpy's cumsum). Now find the max and min value. And determine an underlying numpy type that is suitable. Now create a new array (or change the params of the existing array if possible) and return this. 

The same would hold for cumprod (but leaving the values at the masked position to be 1).

cummax and cummin would not have this issue of exceeding the bounds of the dtype
Replacing masked values with 0 (or 1 for prod) seems reasonable. We'll need to decide if we want to match NumPy's behavior (which silently overflows) or whether we want to catch that. Either is fine by me.

> Compute the cumsum (using numpy's cumsum). Now find the max and min value.

I don't know that that'll work, because NumPy silently overflows.
On the _data of an extension array, cumsum does not flow over silently, but does the correct aggregation. For the dtype parameter in the cumsum function, the documentation states:


>Type of the returned array and of the accumulator in which the elements are summed. If dtype is  not specified, it defaults to the dtype of a, unless a has an integer dtype with a precision less than  that of the default platform integer. In that case, the default platform integer is used.

So apparently, an easy implementation would be to always return `Int64 Dtype`, which is not optimal but I think a suitable solution at least at the beginning.
Okay, this should be a rough idea in the happy cases. Right now, the tests still fail since I haven't connected `._accumulate(name="cumsum")` to the cumsum function of the Series
@datajanko is this still active?
Yes in fact it is, but unfortunately time is scarce for me currently. Should be better by the end of February. If you want to allocate more/different resources on this issue, I'm of course fine with that.
@datajanko still active? If so can you fix up and try to get green?
Hello @datajanko! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:

































There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2021-02-20 07:51:35 UTC
@WillAyd 
yes, I'm working on this. Should soon be able to have some green tests. I realized that my tests were just wrong.

A different related question: I have a series and call e.g. cumsum on the underlying extension array. What is the recommended way to turn this ExtensionArray again into a pd.Series?
> What is the recommended way to turn this ExtensionArray again into a pd.Series?

`pd.Series(arr)`
Hey, I finally seem to habe a working solution (no tests yet, and somehow the pre-commits currently fail on my system) but in a jupyter notebook I seem to get good results for data generate by

```
def make_data():
    return list(range(1, 9)) + [np.nan] + list(range(10, 98)) + [np.nan] + [99, 100]
```

However, these data are not suitable for `cumprod` due to integer overflows. Is there any suggestion on how to test this? Is there any other comment? Is using `na_accum_func` okay?

How do we want to set the further scope of the ticket? Next step for me is finalizing the tests for Integer Arrays. Sorry for the long time, I didn't expect to dive so deep into pandas internals. But I learned a lot here.

@jbrockmendel I fear you misunderstood my question as being a general question. In the situation at hand `self._constructor` is doing this. However, I simply overlooked this further down in the code and besides was not fully understanding what was happening here. 
so to test the cumulative product, I decided to just take the first 20 rows of the test. A suggestion to organize the tests differently would be welcome. What about the NoAccumulateTests?

About TypeHints, would we want to add `from __future__ import annotations` to allow a typehint for `_accumulate -> ExtensionArray`?

Moreover, the failing tests are just for categorically raising the wrong exception. I'd like to know if the overall direction of the chosen approach is okay
Haven't looked at the rest yet, but

> About TypeHints, would we want to add from __future__ import annotations to allow a typehint for _accumulate -> ExtensionArray?

Our oldest python doesn't support that. You can have the return value be `-> 'ExtensionArray'` (with the quotes).
Sorry, I just realized, that I'm not actually calling the `_accumulate`-function in the respective extension arrays but rather put the burden of accumulation in the na_accumulate_function.

I'll reiterate on that. sorry for the inconvenience
Okay, so I was able do actually delegate the execution to the extension arrays. 

Open points that remain:
- is the approach okay?
- how shall the scope of the ticket be? Now I know what to do at least :)
- Testing of cumprod seems awkward
- I'm doing some kind of integration test by only looking at series and not on extension arrays directly. Shall this be changed?
Any idea on why (except the docs) the tests are failing? Is this a common issue?
Concerning boolean arrays: Would it make sense, in principal to implement them as a subclass of integer arrays and just add additional functions specific to boolean arrays? For the accumulation function - at least - we wouldn't need to write any additional code here
can you merge master and we'll see where we can go with this
Yes, will do it this week and probably also some refactoring. Sorry for the delay
ok, master should be merged. I'd suggest to move `_accumulate` also to masked array as @TomAugspurger 

The, I'd suggest to just push this an not implement the accumulation for other extension arrays, at least for now. What do you think?
On pandas-dev I'm seeing failing tests for date time arrays and cummin/cummax. My feeling is that having at least these two functions there is necessary. Raising a not implemented error would be strange
> On pandas-dev I'm seeing failing tests for date time arrays and cummin/cummax. My feeling is that having at least these two functions there is necessary. Raising a not implemented error would be strange

Can you post a representative traceback?
sure:

<details> 
    <summary>
[Link to CI run](https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=37916&view=logs&j=a67b4c4c-cd2e-5e3c-a361-de73ac9c05f9&t=33d2fdd0-c376-5f94-e6d3-957bdd23a3b8&l=134)
</summary>

```
2020-06-22T20:30:00.2882223Z =================================== FAILURES ===================================
2020-06-22T20:30:00.2882733Z __________ TestSeriesCumulativeOps.test_cummin_datetime64[US/Pacific] __________
2020-06-22T20:30:00.2883859Z [gw0] linux -- Python 3.6.1 /home/vsts/miniconda3/envs/pandas-dev/bin/python
2020-06-22T20:30:00.2884308Z 
2020-06-22T20:30:00.2884683Z self = <pandas.tests.series.test_cumulative.TestSeriesCumulativeOps object at 0x7f7a18548080>
2020-06-22T20:30:00.2893004Z tz = 'US/Pacific'
2020-06-22T20:30:00.2893353Z 
2020-06-22T20:30:00.2893681Z     @pytest.mark.parametrize("tz", [None, "US/Pacific"])
2020-06-22T20:30:00.2894105Z     def test_cummin_datetime64(self, tz):
2020-06-22T20:30:00.2894806Z         s = pd.Series(
2020-06-22T20:30:00.2895328Z             pd.to_datetime(
2020-06-22T20:30:00.2896639Z                 ["NaT", "2000-1-2", "NaT", "2000-1-1", "NaT", "2000-1-3"]
2020-06-22T20:30:00.2897353Z             ).tz_localize(tz)
2020-06-22T20:30:00.2897669Z         )
2020-06-22T20:30:00.2898139Z     
2020-06-22T20:30:00.2898446Z         expected = pd.Series(
2020-06-22T20:30:00.2898773Z             pd.to_datetime(
2020-06-22T20:30:00.2899442Z                 ["NaT", "2000-1-2", "NaT", "2000-1-1", "NaT", "2000-1-1"]
2020-06-22T20:30:00.2899940Z             ).tz_localize(tz)
2020-06-22T20:30:00.2900395Z         )
2020-06-22T20:30:00.2900741Z >       result = s.cummin(skipna=True)
2020-06-22T20:30:00.2901024Z 
2020-06-22T20:30:00.2901503Z pandas/tests/series/test_cumulative.py:79: 
2020-06-22T20:30:00.2902877Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2020-06-22T20:30:00.2903664Z pandas/core/generic.py:11500: in cum_func
2020-06-22T20:30:00.2904077Z     result = self._mgr.apply(block_accum_func)
2020-06-22T20:30:00.2904464Z pandas/core/internals/managers.py:400: in apply
2020-06-22T20:30:00.2905040Z     applied = b.apply(f, **kwargs)
2020-06-22T20:30:00.2905415Z pandas/core/internals/blocks.py:339: in apply
2020-06-22T20:30:00.2905793Z     result = func(self.values, **kwargs)
2020-06-22T20:30:00.2906204Z pandas/core/generic.py:11493: in block_accum_func
2020-06-22T20:30:00.2906946Z     result = values._accumulate(name, skipna, **kwargs)
2020-06-22T20:30:00.2907419Z _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
2020-06-22T20:30:00.2908196Z 
2020-06-22T20:30:00.2908999Z self = <DatetimeArray>
2020-06-22T20:30:00.2909897Z [                      'NaT', '2000-01-02 00:00:00-08:00',
2020-06-22T20:30:00.2910729Z                        'NaT', '2000-01-01 00:00:00-08:00',
2020-06-22T20:30:00.2911557Z                        'NaT', '2000-01-03 00:00:00-08:00']
2020-06-22T20:30:00.2912079Z Length: 6, dtype: datetime64[ns, US/Pacific]
2020-06-22T20:30:00.2912893Z name = 'cummin', skipna = True, kwargs = {}
2020-06-22T20:30:00.2913481Z 
2020-06-22T20:30:00.2914198Z     def _accumulate(self, name, skipna=True, **kwargs) -> "ExtensionArray":
2020-06-22T20:30:00.2914707Z         """
2020-06-22T20:30:00.2915076Z         Return an ExtensionArray performing the accumulation operation.
2020-06-22T20:30:00.2916163Z         The underlying data type might change
2020-06-22T20:30:00.2916681Z         # TODO Clarify
2020-06-22T20:30:00.2917035Z     
2020-06-22T20:30:00.2917308Z         Parameters
2020-06-22T20:30:00.2917986Z         ----------
2020-06-22T20:30:00.2918384Z         name : str
2020-06-22T20:30:00.2918777Z             Name of the function, supported values are:
2020-06-22T20:30:00.2919227Z             # TODO Add function signatures
2020-06-22T20:30:00.2919772Z             - cummin
2020-06-22T20:30:00.2920340Z             - cummax
2020-06-22T20:30:00.2920933Z             - cumsum
2020-06-22T20:30:00.2921510Z             - cumprod
2020-06-22T20:30:00.2921936Z         skipna : bool, default True
2020-06-22T20:30:00.2922304Z             If True, skip NA values.
2020-06-22T20:30:00.2922641Z         **kwargs
2020-06-22T20:30:00.2923031Z             Additional keyword arguments passed to the accumulation function.
2020-06-22T20:30:00.2924139Z             # TODO check if kwargs are needed
2020-06-22T20:30:00.2924642Z             Currently, there is no supported kwarg.
2020-06-22T20:30:00.2924971Z     
2020-06-22T20:30:00.2927829Z         Returns
2020-06-22T20:30:00.2928872Z         -------
2020-06-22T20:30:00.2929456Z         array
2020-06-22T20:30:00.2929734Z     
2020-06-22T20:30:00.2929993Z         Raises
2020-06-22T20:30:00.2930493Z         ------
2020-06-22T20:30:00.2930925Z         TypeError : subclass does not define accumulations
2020-06-22T20:30:00.2931644Z         """
2020-06-22T20:30:00.2932013Z >       raise TypeError(f"cannot perform {name} with type {self.dtype}")
2020-06-22T20:30:00.2932584Z E       TypeError: cannot perform cummin with type datetime64[ns, US/Pacific]
2020-06-22T20:30:00.2932962Z 
2020-06-22T20:30:00.2933262Z pandas/core/arrays/base.py:1121: TypeError
```
</details>
I can reproduce these errors locally.
Interestingly, I only see errors when using a timezone (and not naive date time objects).