API: Expanded resample
State: closed

- [x] closes #13500
- [x] tests added / passed
- [x] passes `git diff upstream/master | flake8 --diff`
- [x] whatsnew entry

My only question here is if `on=` is the keyword name for picking a column - in a `TimeGrouper` that's called `key`.  But `on` is what was used in the rolling selection (and elsewhere) so seems consistent.

```
In [63]: df = pd.DataFrame({'date': pd.date_range('2015-01-01', freq='W', periods=5),
   ....:                    'a': np.arange(5)},
   ....:                   index=pd.MultiIndex.from_arrays([
   ....:                            [1,2,3,4,5],
   ....:                            pd.date_range('2015-01-01', freq='W', periods=5)],
   ....:                        names=['v','d']))
   ....: 

In [64]: df
Out[64]: 
              a       date
v d                       
1 2015-01-04  0 2015-01-04
2 2015-01-11  1 2015-01-11
3 2015-01-18  2 2015-01-18
4 2015-01-25  3 2015-01-25
5 2015-02-01  4 2015-02-01

In [65]: df.resample('M', on='date').sum()
Out[65]: 
            a
date         
2015-01-31  6
2015-02-28  4

In [66]: df.resample('M', level='d').sum()
Out[66]: 
            a
d            
2015-01-31  6
2015-02-28  4
```


Labels:
name: Enhancement
name: Resample
description: resample method

Comments:
would be ok with deprecating 'key' in favor of 'on' for Timegrouper as well

## [Current coverage](https://codecov.io/gh/pydata/pandas/pull/13961?src=pr) is 85.27% (diff: 83.33%)

> Merging [#13961](https://codecov.io/gh/pydata/pandas/pull/13961?src=pr) into [master](https://codecov.io/gh/pydata/pandas/branch/master?src=pr) will decrease coverage by **0.02%**

``` diff
@@             master     #13961   diff @@
==========================================
  Files           139        139          
  Lines         50164      50510   +346   
  Methods           0          0          
  Messages          0          0          
  Branches          0          0          
==========================================
+ Hits          42788      43071   +283   
- Misses         7376       7439    +63   
  Partials          0          0          
```

> Powered by [Codecov](https://codecov.io?src=pr). Last update [257ac88...10c7280](https://codecov.io/gh/pydata/pandas/compare/257ac884d61a74990d1cb4d72c48b1c9003298d5...10c728055df5a1ec6bc3478d535d552d2262423c?src=pr)

API design question: when using `on`, should that columns be set as the index, or left as a column? (for rolling, we kept it as a column, but of course, there the original index is left intact, which is not the case for resample)

@jorisvandenbossche I don't think we actually set the index for anything that accepts `key/on` (`.groupby` / `.merge`) ATM, nor do I think we should. That's exactly the point, the index setting needs to be explicit.

But this will have the index set in the results.  I do think that's the right thing to do - consistent with how `groupby` works elsewhere  It'd be nice if this was an option, but doesn't seem implemented:

```
In [11]: df.groupby(pd.TimeGrouper(key='date', freq='M'), as_index=False).sum()
---------------------------------------------------------------------------

AttributeError: 'BinGrouper' object has no attribute 'compressed'
```

I actually don't have a big problem with the `key` / `on` inconsistency.  They do sort of represent different things - `key` is part of an abstract mapping, where `on` is a selection out of a concrete object?

@chris-b1 I _don't_ think we set when `on` is indicated (in merging), and doesnt' exist in `.groupby` / `.resample` ATM (except via `key`). So can you show an example.

I'm probably not being clear.  By "setting" what I mean is that we return this:

```
In [19]: df.resample('M', on='date').sum()
Out[19]: 
            a
date         
2015-01-31  6
2015-02-28  4
```

Instead of this - i.e., the `on` column becomes an index.

```
In [20]: df.resample('M', on='date').sum().reset_index()
Out[20]: 
        date  a
0 2015-01-31  6
1 2015-02-28  4

```

@chris-b1 I understand. And I think the point IS to return `[20]`. we had this same discussion in #13358. 

Certainly there is a case for this. E.g. Time is the grouper here, so we _should_ set the index. And maybe this _is_ different than the merging case rationale. Counterpoint is that the point of `on` is to indicate a grouper that we _don't_ want to set in the first place (eg. this defeats the purpose of the `.set_index(on_column).resample(...).reset_index(on_column)` idiom.

Just want to clearly delineate cases where we _should_ and _should not_ set the resulting index.
I think having a table (in the dev docs?) where the philosophy / rationale would be good. (or can just lay it out here is fine too).

xref #5755 - whether this is exactly what _should_ be done, the basic rules now seem to be:
- Transformations (`sort`, `rolling`, `groupby.transform`) - never set index
- Reductions (`df.sum`, `df.groupby.sum`) - set index to reducing grouping.  In some places (`groupby` is all I can think of) you have an option to make the grouping column rather than an index
- Relational Ops
  - index based (`concat`, `join`, `align`) - set index to resulting set
  - column based (`merge_*`) - discard existing index

Small updates pushed for the comments.  I'd propose the that setting the `on` as an index (current impl) would be the most consistent API design, but certainly can change that if you feel otherwise.

@chris-b1 With the current PR, if you have a non-reducing method, you get the following:

```
In [4]: df.resample('M', on='date').transform(lambda x: x)
Out[4]: 
   a
0  0
1  1
2  2
3  3
4  4
```

Which is I think not what we want? (the 'date' column should still be there?)

I think either we should follow the logic from groupby (reducing -> set as index (current PR for reducing methods), transforming -> keep as column) or either keep it as column in all cases (to distinguish it as use case from `set_index('date').resample(..)`

You must not be using the same frame as my example?  I show the index always being preserved, which is expected.   I could see an argument that `'date'` should be preserved as a column in `[3]`,

```
In [1]: pd.__version__
Out[1]: '0.18.1+345.gc4db0e7'

In [2]: df
Out[2]: 
              a       date
v d                       
1 2015-01-04  0 2015-01-04
2 2015-01-11  1 2015-01-11
3 2015-01-18  2 2015-01-18
4 2015-01-25  3 2015-01-25
5 2015-02-01  4 2015-02-01

In [3]: df.resample('M', on='date').transform(lambda x: x)
Out[3]: 
              a
v d            
1 2015-01-04  0
2 2015-01-11  1
3 2015-01-18  2
4 2015-01-25  3
5 2015-02-01  4

In [4]: df.resample('M', level='d').transform(lambda x: x)
Out[4]: 
              a       date
v d                       
1 2015-01-04  0 2015-01-04
2 2015-01-11  1 2015-01-11
3 2015-01-18  2 2015-01-18
4 2015-01-25  3 2015-01-25
5 2015-02-01  4 2015-02-01

```

@chris-b1 ah yes sorry, I used the same example dataframe, but without the index. It's because the index is identical to the column in your example that the results 'looks' good I think (because the index including the same values as the dropped column is still there):

```
In [63]: df2 = pd.DataFrame({'date': pd.date_range('2015-01-01', freq='W', periods=5),
   ....:                     'a': np.arange(5)})

In [64]: df2.resample('M', on='date').transform(lambda x: x)
Out[64]: 
   a
0  0
1  1
2  2
3  3
4  4
```

I actually still think the current approach may be right.  Is it any different than `'a'` being excluded from the results here?

```
In [11]: df = pd.DataFrame({'a': [1, 1, 2, 2], 'b': [1, 2, 3, 4]})

In [12]: df.groupby('a').transform(lambda x: x)
Out[12]: 
   b
0  1
1  2
2  3
3  4
```

Hmm, OK, I was in the belief that that preserved the 'a' column, apparantly not ... Oh, what a jumble this grouping api :-) 
Other 'transforming' methods keep the grouping column (eg `ffill` or `head`)

EDIT: after some thought, it is indeed useful `transform` does not preserve the grouping column, this way assigning the result to a new column of the original dataframe is much easier.

I think this is a fine extension of the API. It is in-line with other methods of how it produces output. 

Ok, one (hopefully) last api question.   

Right now this blows up in various ways if an upsample is attempted, e.g. `df.resample('D', level='d').ffill()`.  _Maybe_ that's a valid use case, but it's a lot less compelling to me, and doesn't map to groupby semantics, so what I'm proposing is to raise `NotImplementedError` for now, 

@jreback - expanded tests like you suggested, moving as much the api to the `Base` class as possible, so all three index types are hit.

This also now closes #14008, it's kind of a hack, but not sure there's a much better way without rethinking the whole approach, xref #12884.

I'm using the `NotImplementedError` approach for up-sampling suggested above.

pls remove the 'hack'. make this PR simpler.

I'm very open to suggestions, but this is as clean of approach as I can think of.  I can always back out changes and just leave #14008 open if the fix is worse than the bug.

yes pls back out

the period resampling needs to be fixed in s more core way

If you'd like to have another look, I've removed the `PeriodIndex` changes.  I did leave in the `from_selection` state (may be a better name for this? though internal only), but it's only used to raise sensible errors for not-implemented things.

@jreback - updated for that last comment, let me know if you see anything else.

thanks @chris-b1