Series 'pct_change' with 'fill_method' broken

[WillAyd]

The fill_method argument for pct_change does not have any effect on the calculated result. To illustrate:

>>> ser = pd.Series([np.nan, np.nan, 0, 2, 4, 6, np.nan, np.nan])
>>> ser.pct_change()
0         NaN
1         NaN
2         NaN
3         inf
4    1.000000
5    0.500000
6         NaN
7         NaN
dtype: float64

    # Will return same result as above
>>> ser.pct_change(fill_method='pad')
0         NaN
1         NaN
2         NaN
3         inf
4    1.000000
5    0.500000
6         NaN
7         NaN
dtype: float64

I would expect rows 6 and 7 above to be 0 as a result of the fill operation. The problem can be traced to the below line of code, where the mask is referencing the original values before any fill logic is applied:

pandas/pandas/core/generic.py

Line 6893 in a00154d

mask = isna(_values_from_object(self))

INSTALLED VERSIONS

commit: ce77b79
python: 3.6.1.final.0
python-bits: 64
OS: Darwin
OS-release: 17.4.0
machine: x86_64
processor: i386
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.23.0.dev0+364.gce77b79b9.dirty
pytest: None
pip: 9.0.1
setuptools: 27.2.0
Cython: None
numpy: 1.13.1
scipy: 0.19.1
pyarrow: None
xarray: None
IPython: 6.1.0
sphinx: 1.6.5
patsy: None
dateutil: 2.6.1
pytz: 2017.2
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.1.1
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: 4.6.0
html5lib: 0.999999999
sqlalchemy: 1.1.13
pymysql: None
psycopg2: 2.7.3.2 (dt dec pq3 ext lo64)
jinja2: 2.9.6
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

[WillAyd]

FWIW fill_method defaults to pad without specifying the keyword argument so both calls above are identical, but the point remains that no filling action actually occurs to return 0 instead of NaN where appropriate

[Loupiol]

Hello, I just upgraded recently and noticed this change. To me this change does not make sense, because for a stock price time series, if a price is missing for one day, it should not return 0.0 as pct_change, but should just be empty (can bias some statistics computed on price returns). Would it be possible to add an argument to choose the behavior? To clarify, this should still forward fill by default, but I'd say the user should be able to choose if he considers a missing data as a 0 return day or as a nan return day. Thanks

[WillAyd]

@Loupiol would be better if you open your request in a separate issue and provide code examples of what you expect to see. Can discuss that from there!

[selik]

@WillAyd From the docs (https://pandas.pydata.org/pandas-docs/stable/reference/api/pandas.Series.pct_change.html) v25.1 it looks like fill_method='pad' is the default, so that fill_method=None should behave differently. In fact, I argue in #6389 that fill_method=None should be the default behavior. I think the pad default behavior is a UX bug.