Repository: pandas-dev/pandas
Pull request: DOC: warning on look-ahead bias with resampling
- [x] closes #11123
- [ ] tests added / passed
- [ ] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Problem statement:
Business day resampling 
Hi everyone, 

I just stumbled across this odd behaviour: 

For a daily time series, I would expect `resample('B',  how='last')` to be equivalent to selecting rows whose weekday is not Saturday or Sunday.  However, `resample` groups together Fridays,  Saturdays and Sundays and hence we get Sunday's value on Friday.

In the time series context, I guess most people would expect `resample('B', ...)` to bin Saturdays, Sundays and Mondays, so that **no looking into the future** occurs.

Am I missing something?

Hints from issue discussion:
pls show a specific example

``` python
In [2]: pd.__version__
Out[2]: '0.16.2'

In [3]: s = pd.Series(list(range(10)), pd.date_range('2015-09-01', '2015-09-10'))

In [4]: s
Out[4]: 
2015-09-01    0
2015-09-02    1
2015-09-03    2
2015-09-04    3  <--- Friday
2015-09-05    4
2015-09-06    5
2015-09-07    6  <--- Monday
2015-09-08    7
2015-09-09    8
2015-09-10    9
Freq: D, dtype: int64

In [5]: s.resample('B', how='last')
Out[5]: 
2015-09-01    0
2015-09-02    1
2015-09-03    2
2015-09-04    5  <--- expected 3
2015-09-07    6
2015-09-08    7
2015-09-09    8
2015-09-10    9
Freq: B, dtype: int64
```

see also #11128 

```
In [34]: import calendar
In [35]: weekdays = [calendar.day_name[i].lower() for i in range(7)]
In [36]: s = pd.Series(list(range(10)), pd.date_range('2015-09-01', '2015-09-10'))
In [37]: s2 = s.to_frame('value').assign(weekday=Series(s.index.weekday).map(Series(weekdays)).values)

In [38]: s2
Out[38]: 
            value    weekday
2015-09-01      0    tuesday
2015-09-02      1  wednesday
2015-09-03      2   thursday
2015-09-04      3     friday
2015-09-05      4   saturday
2015-09-06      5     sunday
2015-09-07      6     monday
2015-09-08      7    tuesday
2015-09-09      8  wednesday
2015-09-10      9   thursday
```

This is equivalent to `.resample('B')`

```
In [39]: s2.groupby(pd.Grouper(freq='B')).last()
Out[39]: 
            value    weekday
2015-09-01      0    tuesday
2015-09-02      1  wednesday
2015-09-03      2   thursday
2015-09-04      5     sunday
2015-09-07      6     monday
2015-09-08      7    tuesday
2015-09-09      8  wednesday
2015-09-10      9   thursday
```

You can do this

```
In [40]: def f(x):
    # x is a Timestamp
    if x.weekday() < 5:
        return x
    return x + pd.offsets.BusinessDay(0)
   ....: 

In [41]: s2.groupby(f).last()
Out[41]: 
            value    weekday
2015-09-01      0    tuesday
2015-09-02      1  wednesday
2015-09-03      2   thursday
2015-09-04      3     friday
2015-09-07      6     monday
2015-09-08      7    tuesday
2015-09-09      8  wednesday
2015-09-10      9   thursday
```

I think you should need to create another business day offset to actually do this with a freq. E.g. you could sub-class `BusinessDay` and create 2 new offsets, e.g. `BF` and `BM` (or somesuch), where `BF` would be the same as `BusinessDay`. So i'll mark this as an enhancement.

I didn't dig into the code yet, but maybe we can use the keyword arg `convention` ?

On a more general level, I'm not sure the present behaviour should be the default one. It really doesn't feel right to have a peek into the future in the time series context.

you misunderstand, you would need to create a new frequency. This is a long time convention. You could add to, but not change this.

However,

``` python
In [35]: s2.to_period('B')
Out[35]: 
            value    weekday
2015-09-01      0    tuesday
2015-09-02      1  wednesday
2015-09-03      2   thursday
2015-09-04      3     friday
2015-09-07      4   saturday
2015-09-07      5     sunday
2015-09-07      6     monday
2015-09-08      7    tuesday
2015-09-09      8  wednesday
2015-09-10      9   thursday
```

groups together Saturdays, Sundays and Mondays which is the sane default behaviour IMO.

Don't you agree this is a bit inconsistent ?

@0x0L you are talking about 2 different things here. `periods` are timespans which encompass a start-end. `timestamps` are points-in-time. conversions between one and another must use a convention (e.g. start/end).

However, resample is a point-in-time operation (at least on a DatetimeIndex). So the issue is what convention to use. The existing convention is to include fri-sat-sun as indicated. 

doing `to_period` just marks the intervals, and is NOT a resample

@jreback I understand. That's what I was trying to say when I earlier mentioned we could use the convention keyword arg. Right now,

``` python
In [10]: x = s2.resample('B', how='last', convention='start')

In [11]: y = s2.resample('B', how='last', convention='end')
```

are the same. Why not use it to make `resample` group sat-sun-mon ?

convention is implied by the freq for a time-stamp, e.g `B` -> implies `include fri-sat-sunday`. So its a part of the frequency, not a keyword. A keyword is needed however for `Period` because that is a Timespan and it is ambiguous which frequency to go to.

Commit message: Note on look-ahead bias with resampling

Patch to review:
diff --git a/doc/source/user_guide/timeseries.rst b/doc/source/user_guide/timeseries.rst
index f559b0d073320..0c6c94c16f4e5 100644
--- a/doc/source/user_guide/timeseries.rst
+++ b/doc/source/user_guide/timeseries.rst
@@ -761,34 +761,6 @@ regularity will result in a ``DatetimeIndex``, although frequency is lost:
 
    ts2[[0, 2, 6]].index
 
-.. _timeseries.iterating-label:
-
-Iterating through groups
-------------------------
-
-With the ``Resampler`` object in hand, iterating through the grouped data is very
-natural and functions similarly to :py:func:`itertools.groupby`:
-
-.. ipython:: python
-
-   small = pd.Series(
-       range(6),
-       index=pd.to_datetime(['2017-01-01T00:00:00',
-                             '2017-01-01T00:30:00',
-                             '2017-01-01T00:31:00',
-                             '2017-01-01T01:00:00',
-                             '2017-01-01T03:00:00',
-                             '2017-01-01T03:05:00'])
-   )
-   resampled = small.resample('H')
-
-   for name, group in resampled:
-       print("Group: ", name)
-       print("-" * 27)
-       print(group, end="\n\n")
-
-See :ref:`groupby.iterating-label` or :class:`Resampler.__iter__` for more.
-
 .. _timeseries.components:
 
 Time/Date Components
@@ -1628,24 +1600,29 @@ labels.
 
    ts.resample('5Min', label='left', loffset='1s').mean()
 
-.. note::
+.. warning::
 
-    The default values for ``label`` and ``closed`` is 'left' for all
+    The default values for ``label`` and ``closed`` is '**left**' for all
     frequency offsets except for 'M', 'A', 'Q', 'BM', 'BA', 'BQ', and 'W'
     which all have a default of 'right'.
 
+    This might lead to unintended look-ahead bias as in the following example:
+
     .. ipython:: python
 
-       rng2 = pd.date_range('1/1/2012', end='3/31/2012', freq='D')
-       ts2 = pd.Series(range(len(rng2)), index=rng2)
+        s = pd.date_range('2000-01-01', '2000-01-05').to_series()
+        s.iloc[2] = pd.NaT
+        s.dt.weekday_name
 
-       # default: label='right', closed='right'
-       ts2.resample('M').max()
+        # default: label='left', closed='left'
+        s.resample('B').last().dt.weekday_name
 
-       # default: label='left', closed='left'
-       ts2.resample('SM').max()
+    Notice how the value for Sunday got pulled back to the previous Friday.
+    To prevent any look-ahead bias, use instead
 
-       ts2.resample('SM', label='right', closed='right').max()
+    .. ipython:: python
+
+        s.resample('B', label='right', closed='right').last().dt.weekday_name
 
 The ``axis`` parameter can be set to 0 or 1 and allows you to resample the
 specified axis for a ``DataFrame``.
@@ -1796,6 +1773,34 @@ level of ``MultiIndex``, its name or location can be passed to the
 
    df.resample('M', level='d').sum()
 
+.. _timeseries.iterating-label:
+
+Iterating through groups
+~~~~~~~~~~~~~~~~~~~~~~~~
+
+With the ``Resampler`` object in hand, iterating through the grouped data is very
+natural and functions similarly to :py:func:`itertools.groupby`:
+
+.. ipython:: python
+
+   small = pd.Series(
+       range(6),
+       index=pd.to_datetime(['2017-01-01T00:00:00',
+                             '2017-01-01T00:30:00',
+                             '2017-01-01T00:31:00',
+                             '2017-01-01T01:00:00',
+                             '2017-01-01T03:00:00',
+                             '2017-01-01T03:05:00'])
+   )
+   resampled = small.resample('H')
+
+   for name, group in resampled:
+       print("Group: ", name)
+       print("-" * 27)
+       print(group, end="\n\n")
+
+See :ref:`groupby.iterating-label` or :class:`Resampler.__iter__` for more.
+
 
 .. _timeseries.periods:
 


Code review comments:
diff_hunk: @@ -1628,24 +1600,29 @@ labels.
 
    ts.resample('5Min', label='left', loffset='1s').mean()
 
-.. note::
+.. warning::
 
-    The default values for ``label`` and ``closed`` is 'left' for all
+    The default values for ``label`` and ``closed`` is '**left**' for all
     frequency offsets except for 'M', 'A', 'Q', 'BM', 'BA', 'BQ', and 'W'
     which all have a default of 'right'.
 
+    This might lead to unintended look-ahead bias as in the following example:
path: doc/source/user_guide/timeseries.rst
text: @user1:
Is "look-ahead bias" a common term?

@author:
It is in the context of timeseries forecasting.
EDIT: what about dropping the word `bias` ?

@user1:
Yes, dropping bias, and perhaps adding a small definition sounds good. Something like

    This might lead to unintended looking ahead, where the value for a later date (Sunday) is pulled back to
    a previous date (Friday).