ENH: Add the moment function as DataFrame and Series method WITH namespacing
State: closed

This makes the usage of those methods more object oriented

Also, delete some trailing whitespaces.


Labels:
name: Docs
name: API Design
name: Numeric Operations
description: Arithmetic, Comparison, and Logical operations
name: Needs Discussion
description: Requires discussion from core team before further action

Comments:
ok, this looks reasonable. 
- I would like to have some smoke tests along the lines of  [this](https://github.com/pydata/pandas/blob/master/pandas/tests/test_groupby.py#L4841). 
- pls add to the `API.rst` as well (maybe have to do that somewhat manually).
- Further I think that `computation.rst` could be modified to use this syntax.
- finally would need a release note showing these changes

While in principle I agree that it would be consistent to have these on the objects, I am just not sure if I find it a good idea to add yet another _37_ methods at once to DataFrame/Series.

I share @jorisvandenbossche's concern. We _could_ do it in a `.rolling` and `.expanding` namespace. `df.rolling.mean(periods=2)`, but I don't have a strong preference for that over where they are now.

ok, these are good points. Not real strong on this, so tabling for now, @TomAugspurger of namespacing makes more sense. 

I like the idea of adding a namespace to avoid adding 37 names into DataFrame.

IMHO, there are two clean ways of doing this:
1. create on the fly the rolling and expanding namespaces in DataFrame, something close to what is done here https://github.com/dalejung/trtools/blob/master/trtools/core/rolling.py (though I must admit I did not take the time to understand in detail this solution)
2. change the moments library to gather all the rolling methods in a rolling namespace and all the expanding methods in an expanding namespace

I prefer the solution 2 since it would also make the moments library cleaner.

I am not very comfortable with pandas' code yet. Could you tell me whether one of those ideas could be acceptable or if another solution could be preferable?

I gathered the rolling and expanding methods into separate namespaces and added them to DataFrame and Series

This way, the dataframes methods are available via: DataFrame.rolling._,
DataFrame.expanding._, DataFrame.ewma. Same for Series.

Considering the ewm\* function, only 8 objects are added to the DataFrames. I did
not not whether it was a good idea to put the ewm\* method in a separate
namespace.

Hmm, the last implementation just does not work because df.rolling.mean provides the rolling object to the rolling_mean method as self argument instead of the dataframe. 

In order to use the rolling_ and expanding_ methods as is but still aggregate them into separate namespaces, I don't see an easy solution.

https://github.com/dalejung/trtools/blob/master/trtools/monkey/__init__.py provides a way to do so, but it looks a bit complicated to do a so simple stuff.

@Konubinix We have some machinery for this (which is also used by dt/cat/str). Look for `AccessorProperty` in core/base.py.

But maybe wait a moment to first let some other people react and settle the discussion on which we actually want (as methods, as namespace methods or leave as is) before you put too much work in it.

`.rolling`, `.expaning`, `.ewma` make name spaces for `Series` make sense, though these only should show up for `int/float` dtypes (similar to how `.str/.cat/.dt` work).

For `DataFrame` this is much tricker. You only really would want them to be visible for a single dtype frame (of `int/float`), otherwise they don't make sense. 

I like the idea of namespace accessors for rolling and expanding aggregations. It has always seemed a little strange to me how these are functions instead of methods (like most things in pandas).

I took into account your remarks and comments.
Now the methods are gathered into separate namespaces (ewm, rolling and expanding) and a check is made whether a Serie is of dtype int of float and a DataFrame contains at least one dtype int or float.

@jreback, thank you for the PandasDelegate pointer.

I checked into that possibility.

If I understood correctly, it consists in subclassing PandasDelegate with a
class that performs the computation for each delegated name. Then we have to
call the method class _add_delegate_accessors to indicate from which class to
copy the methods signatures and docstring.

I my situation, since I don't propose to copy some methods from one delegate
class, it looks not really well suited.

For instance, to allow to use df.rolling.mean, I would need to create an
intermediate class RollingMethods with the method mean (avg, count, ...) that
would wrap the rolling_mean (rolling_avg, rolling_count, ...) methods, then
create a delegator class RollingDelegator that would implement the abstract
method _delegate_method inherited from PandasDelegate.

Then, I would have to call RollingDelegator._add_delegate_accessors(delegate=RollingMethods, accessors=[...the list of rolling methods...], typ="method").

Finally, I would have to add to do the same thing as in the previous patch:
create an accessor for the rolling namespace that would check the dtypes and
return an instanciated RollingDelegator.

I would also need to do that for expanding and ewm.

I think I can do this, but it results in much more code that is not more
readable.

I may have understood something wrong. If so, could you please give me some hint on how to use PandasDelegate?

Thanks again for your help.

I pushed a version making use of the PandasDelegate mechanism. I am not sure I use it correctly.

Just to point out another possible API here --

Instead of writing `df.rolling.mean(periods=2)`, we might move the periods argument earlier: `df.rolling(periods=2).mean()`. `df.rolling(periods=2)` could return a `Rolling` object with a groupby like API (e.g., supporting iteration and automatic method dispatch).

Conceptually, this makes some sense -- rolling window operations are very similar to grouped operations with overlapping groups. One possible downside is that methods off an accessor property are more discoverable -- IPython can do direct auto-complete.

@shoyer +1

@shoyer, I like the idea. Nevertheless, it looks way more ambitious than this
pull request.

The purpose of this pull request is to make the functions in the moments
namespace available in the dataframe and series namespaces. It was based on the
observation that all those functions take a dataframe or serie as first argument
and hence are particularly designed to work like instance methods. It then does
no more that inspecting the moments namespace and make its methods accessible
from dataframe and series instances.

You are now suggesting to change the API. I agree with this idea, but I think it
should be the purpose of another issue that could be entitled "Refactor the
moments namespace".

this need a test along the lines of this: https://github.com/pydata/pandas/blob/master/pandas/tests/test_series.py#L79

IOW you are testing that the `__dir__` is correct (and that they execute the same as the `pd.*` functions.

I think your impl is fine. We can discuss doing a `Rolling` in another issue. Its a nice idea but is a pretty big expansion.

@Konubinix @jreback I think this change itself is a great idea, but I'm raising the issue of the alternate API for a few reasons --
1. it's generally better to hold off API changes if they would cause awkward compatibility issues with a different, superior API that we might want to use in the future (that isn't really the case here since `.rolling` as an accessor and as a method can co-exist).
2. from a marketing perspective, it's awkward to make APIs that may only live for a few versions
3. it might not actually be that hard to make a basic `Rolling` object. I would certainly rather have a `Rolling` implementation missing some features we'd like for the final version (e.g., with only support for aggregation for now) than a new API that we might immediately deprecate.

@shoyer, could you be more specific about the missing features with the proposed API?

If in understand correctly, the rolling_XXX(df, window_size, ...) API already exists in pandas.stats.moments. By design, this patch does not create a new API, it only makes the pandas.stats.moments API accessible directly through Dataframe (allowing one to write df.rolling.XXX(window_size, ...) instead of pd.rolling_XXX(df, window_size, ...)).

I have the feeling that any change of API should be done directly in the pandas.stats.moments library. This change would then be almost automatically visible in the Dataframes.

This is the reason why I guess that the change of API using a Rolling object should be done in a separate work. This change would allow the user to write something like pd.rolling(window_size).mean(df)

May be this should be done before making the API available in Dataframes. Unfortunately, I don't think I will have time to code this one.

@shoyer, could you propose something?

@Konubinix a rolling object would be useful for things like this:

```
r = s.rolling(window=10)
r.mean()
r.std()
```

IOW doing a repeated computation on a logical object, very similar to groupby.

The problem is that

`s.rolling` is a nice way of creating a `Rolling` object, and not just a method call, so we'd end up doing this change, then _changing_ again, which is not a great option.

@jreback, in that sense, I suppose it would also be usefull to do things like:

``` python
r = pandas.rolling(s, window=10)
r.mean()
r.std()
```

Similar to the way the groupby function is also accessible via:

``` python
g = pandas.groupby(s, by)
g.mean()
g.std()
```

Am I right?

yes, I suppose those should be supported, although they are pretty non-intuitive. (all these do it call the method on the object anyhow). 

Though if we do change this in the future I would simply deprecate and move to a new more consistent syntax.

I don't think there's any value in supporting calling `pandas.Rolling` directly.

Here's a sketch of what I was thinking:

``` python
class DataFrame:
    def rolling(self, window,  freq=None, center=False, how=None, axis='index'):
        # note that axis is not currently a valid parameter for most rolling aggregations,
        # so perhaps we should skip it for now 
         return Rolling(self, window, freq, center, how, axis)

class Rolling:
    def __init__(self, window, freq, center, how):
        self.window = window
        self.freq = freq
        self.center = center
        self.how = how

    def mean(self, min_periods=None):
        return pd.rolling_mean(self.window, min_periods, self.freq, self.center, self.how)

    # likewise for the median, sum, var, etc.
```

This lets you write things like `df.rolling(window=5).mean()`

The main area where this breaks down is that each of the rolling aggregation functions accepts a `freq` argument for resampling, and uses a different `how` method to control how the resampling is done. I'm not sure it makes sense for different aggregation functions to resample differently if they're all dispatched from the same method.

@shoyer, It sounds like a good idea. Are you sure about the parameters of the rolling property?

Oops, `rolling` should not be a property in my version -- just fixed my comment.

Here's the full set of rolling window functions:
http://pandas.pydata.org/pandas-docs/stable/api.html#standard-moving-window-functions

`rolling_corr` and `rolling_cov` don't fit the same pattern as the others -- they'll need to remain only function calls.

`rolling_window` also has a funny function signature that seems unlikely to fit on the generic `Rolling` without some API adjustments. This is probably worth doing but maybe not immediately necessary.

By reading the code, we can see that the functions that follow the same pattern are specific cases of  _rolling_moment (as created by _rolling_func). They are rolling_max, rolling_min, rolling_sum, rolling_mean, rolling_median, rolling_std, rolling_var, rolling_skew, rolling_kurt.

We could do the same for expanding_XXX but not ewmXXX that have specific code for each case.

Also, I think that rolling_corr and rolling_cov could also take advantage of a more object oriented API.

``` python
df1.rolling(window=50).cov(df2)
```

May be we could at first provide the easy ones rolling_max, rolling_min... in an object oriented way and decide later how to provide the other ones.

@shoyer, how did you come to this API split between **init** and mean? What would be wrong with the following implementation?

``` python
class Rolling:
    def __init__(self, window, freq, center, how, min_period):
        self.window = window
        self.freq = freq
        self.center = center
        self.how = how
        self.min_period = min_period

    def mean(self):
        return pd.rolling_mean(self.window, self.min_periods, self.freq, self.center, self.how)
   def max(self):
         ...
```

May be we could also provide both alternatives

``` python
class Rolling:
    def __init__(self, window, freq, center, how, min_period):
        self.window = window
        self.freq = freq
        self.center = center
        self.how = how
        self.min_period = min_period

    def mean(self, window=None, freq=None, center=None, how=None, min_period=None):
        return pd.rolling_mean(window or self.window, min_periods or self.min_periods, freq or self.freq, center or self.center, how or self.how)
   def max(self, window=None, freq=None, center=None, how=None, min_period=None):
         ...
```

> We could do the same for expanding_XXX but not ewmXXX that have specific code for each case.

This is just a detail of the current implementation. The functions have a very similar signature, so I think they could still be refactored to use a shared object.

> `df1.rolling(window=50).cov(df2)`

My concern with this API for rolling cov is that it's not obvious whether df2 should have the same shape as df1 (which is what I think you intended) or as the window (which is arguably more consistent with how groupby already works). I would rather have something more obviously symmetric, e.g., `df1.rolling(window=50).cov('x', 'y')`.

> @shoyer, how did you come to this API split between init and mean?

The part that controls setting up the window should go in `__init__` and the part the controls how to aggregate should go in `mean`. For example, suppose we make Rolling objects interable like GroupBy objects. Enough information should go in `__init__` that we could interate over each windowed view.

@Konubinix want to repurpose this to generate a `Rolling` grouper-like? (using @shoyer ) model

Seems like a much more natural soln here.

@jreback. Yes I do. In my opinion, it would need to rework partly the stats library to be done properly. Unfortunately, I won't have time to do it myself in the near future. May be @shoyer could propose something?

closing in favor of #11603