BUG: crash in df.groupby.rolling.mean with forward window indexer

[mIgLLL]

• I have checked that this issue has not already been reported.

• I have confirmed this bug exists on the latest version of pandas.

• (optional) I have confirmed this bug exists on the master branch of pandas.

---

Note: Please read this guide detailing how to provide the necessary information for us to reproduce your bug.

Code Sample, a copy-pastable example

# Your code here

Here is a replicate code:
df = pd.DataFrame({'B': [np.nan, 1, 2, np.nan, 4,5,6,7,8,9,10,11,12,13,14,15]})
df['A']=1
indexer = pd.api.indexers.FixedForwardWindowIndexer(window_size=12) # forward 12 period
# First time is OK
df['mean']=df.groupby('A')['B'].rolling(window=indexer, min_periods=1).mean().reset_index().loc[:,'B']
# Second time would crash!
df['mean']=df.groupby('A')['B'].rolling(window=indexer, min_periods=1).mean().reset_index().loc[:,'B']

If don't have groupby, it won't crash.
If don't forward window indexer, it won't crash.
If in the first time, it won't crash.

Problem description

[this should explain why the current behaviour is a problem and why the expected output is a better solution]

When you use df.groupby.rolling.mean with a forward-looking period, it would crash if you run the df.groupby.rolling.mean again.

Expected Output

Output of pd.show_versions()

INSTALLED VERSIONS ------------------ commit : c7f7443 python : 3.8.11.final.0 python-bits : 64 OS : Windows OS-release : 10 Version : 10.0.19042 machine : AMD64 processor : Intel64 Family 6 Model 165 Stepping 2, GenuineIntel byteorder : little LC_ALL : None LANG : zh_CN LOCALE : Chinese (Simplified)_China.936

pandas : 1.3.1
numpy : 1.20.3
pytz : 2021.1
dateutil : 2.8.2
pip : 21.2.2
setuptools : 52.0.0.post20210125
Cython : 0.29.24
pytest : 6.2.4
hypothesis : None
sphinx : 4.0.2
blosc : None
feather : None
xlsxwriter : 3.0.1
lxml.etree : 4.6.3
html5lib : 1.1
pymysql : None
psycopg2 : None
jinja2 : 2.11.3
IPython : 7.26.0
pandas_datareader: None
bs4 : 4.9.3
bottleneck : 1.3.2
fsspec : 2021.07.0
fastparquet : None
gcsfs : None
matplotlib : 3.4.2
numexpr : 2.7.3
odfpy : None
openpyxl : 3.0.7
pandas_gbq : None
pyarrow : None
pyxlsb : None
s3fs : None
scipy : 1.6.2
sqlalchemy : 1.4.22
tables : 3.6.1
tabulate : None
xarray : None
xlrd : 2.0.1
xlwt : 1.3.0
numba : 0.53.1
[paste the output of pd.show_versions() here leaving a blank line after the details tag]

[dsm054]

Yep, I see this too. Even the simpler

df = pd.DataFrame({"A": [1, 1], "B": [0, 1]})
indexer = pd.api.indexers.FixedForwardWindowIndexer(window_size=1)
df.groupby('A')['B'].rolling(window=indexer).mean()
df.groupby('A')['B'].rolling(window=indexer).mean()

segfaults on #2.

If I'm understanding the problem correctly, the indexer is being mutated. Before the first groupby, indexer.window_size is 1; after the first groupby, it's not there at all.

After that, since FixedForwardWindowIndexer is a subclass of BaseIndexer, the first branch in RollingGroupby._get_window_indexer is taken, and we build a GroupbyIndexer which has window=0 and no window_size in indexer_kwargs any more, because it's been popped, and that leads to bad things.

roll_mean isn't happy not getting an end which matches start.

[mzeitlin11]

Thanks for reporting this @mIgLLL! Your investigation makes sense @dsm054, would you be interested in making a pr to fix this?

[mIgLLL]

BTW, I find that the indexer also have another bug.

df = pd.DataFrame({'B': [np.nan, 1, 2, np.nan, 4,5,6,7,8,9,10,11,12,13,14,15,16,17]})
df.loc[:7,'A']=1
df.loc[7:,'A']=2
df['C']=range(18)

indexer = pd.api.indexers.FixedForwardWindowIndexer(window_size=12) # forward 12 period

df['mean']=df.groupby('A')['B'].rolling(window=indexer, min_periods=1).mean().reset_index().loc[:,'B']

df=df.set_index('C')

reverse_mean=df[::-1].groupby('A')['B'].rolling(window=12, min_periods=1).mean()[::-1].reset_index().rename(columns={'B':'mean2'})

df=dd.merge(df,reverse_mean,on=['A','C'],how='left')

If there is a df.groupby, something went wrong! there is unnecessary missing values. And I find the number of missing value in "mean" column is related to the group diviation (df.loc[:7,'A']=1), If you change the size of group 1, the number of missing value also change! But it is not related to the size of group 2.

[dsm054]

@mIgLLL: yeah. It looks to me like the end value (the right side of the bounds it uses for each window) is wrong-- it winds up being too long, which means that the offset is wrong after recombination. We wind up asking roll_mean to act on the values between positions

start: [ 0  1  2  3  4  5  6  7  8  9 10 11 12 13 14 15 16 17] 
end:   [ 7  7  7  7  7  7  7  7  7  7  7  7 18 18 18 18 18 18 18 18 18 18 18 18]

and you can see why you get the five NaN values in the middle. The two arrays aren't even the same length. The logic in FixedForwardWindowIndexer.get_window_bounds is b0rked, and I'm pretty sure I know where and how to fix it. It'll work by accident in isolation, but not when combined with anything else.

@mzeitlin11: sure, I can take a run at it. It's been a while, but I think I still remember how to play. :)

(To be clear, there are two separate bugs here, although both ultimately result in incorrect end arrays. I'm proposing to address both of them.)

[dsm054]

Okay, I think I have a viable fix for both, and new tests have been added to verify the behaviour is sane in this and other cases.

@mzeitlin11: One thing I noticed in passing is that indices is misspelled as "indicies" in a bunch of places, unfortunately including one user-visible location, as an argument to GroupbyIndexer. Does this mean there'd need to be a deprecation period if we change it there, or is GroupbyIndexer sufficiently internal that it can simply be fixed?

[mzeitlin11]

@mzeitlin11: One thing I noticed in passing is that indices is misspelled as "indicies" in a bunch of places, unfortunately including one user-visible location, as an argument to GroupbyIndexer. Does this mean there'd need to be a deprecation period if we change it there, or is GroupbyIndexer sufficiently internal that it can simply be fixed?

Not sure, cc @mroeschke if any thoughts here

[mroeschke]

Only the indexers here are public, https://pandas.pydata.org/pandas-docs/stable/reference/window.html#window-indexer, so other ones, including GroupbyIndexer, can have the spelling mistakes fixed.

Thanks for looking into a fix.