Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

BUG: Documentation not clear or inconsistent with code behaviour: pct_change(fill_method=None) #55351

Closed
3 tasks done
MMCMA opened this issue Oct 2, 2023 · 4 comments
Closed
3 tasks done
Labels
Bug Transformations e.g. cumsum, diff, rank Warnings Warnings that appear or should be added to pandas

Comments

@MMCMA
Copy link

MMCMA commented Oct 2, 2023

Pandas version checks

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

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

  • I have confirmed this bug exists on the main branch of pandas.

Reproducible Example

import pandas as pd
a = pd.DataFrame(dict(a=[1, None, 2]))
a.pct_change()
     a
0  NaN
1  0.0
2  1.0

a.pct_change(fill_method=None)
    a
0 NaN
1 NaN
2 NaN

Issue Description

I was attempting to ensure our code complies with version 2.1 and noticed that the documentation for pct_change() indicates that the fill_method argument has been deprecated since version 2.1. However, the code still executes without a warning or error. Could someone please provide clarification on whether the documentation is incorrect, or if I am misunderstanding something? In the event that this argument will be deprecated in the future, how can I replicate the behavior of .pct_change(fill_method=None) in previous versions? The current default behavior simply carries forward the previous values and calculates percentage changes. Wouldn't this be a scenario where users should use ffill() beforehand?

This relates to #53491, not sure if is a duplicate.

grafik

Expected Behavior

NA

Installed Versions

INSTALLED VERSIONS

commit : e86ed37
python : 3.10.12.final.0
python-bits : 64
OS : Windows
OS-release : 10
Version : 10.0.19045
machine : AMD64
processor : Intel64 Family 6 Model 142 Stepping 12, GenuineIntel
byteorder : little
LC_ALL : None
LANG : None
LOCALE : English_United States.1252
pandas : 2.1.1
numpy : 1.24.4
pytz : 2023.3.post1
dateutil : 2.8.2
setuptools : 68.1.2
pip : 23.2.1
Cython : 3.0.2
pytest : 7.4.2
hypothesis : None
sphinx : None
blosc : None
feather : None
xlsxwriter : None
lxml.etree : 4.9.3
html5lib : 1.1
pymysql : None
psycopg2 : 2.9.6
jinja2 : 3.1.2
IPython : 8.15.0
pandas_datareader : 0.10.0
bs4 : 4.12.2
bottleneck : None
dataframe-api-compat: None
fastparquet : None
fsspec : None
gcsfs : None
matplotlib : 3.7.2
numba : 0.57.1
numexpr : None
odfpy : None
openpyxl : 3.1.2
pandas_gbq : None
pyarrow : 13.0.0
pyreadstat : None
pyxlsb : None
s3fs : None
scipy : 1.10.1
sqlalchemy : 2.0.20
tables : None
tabulate : 0.9.0
xarray : None
xlrd : 2.0.1
zstandard : None
tzdata : 2023.3
qtpy : None
pyqt5 : None

@MMCMA MMCMA added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Oct 2, 2023
@rhshadrach
Copy link
Member

rhshadrach commented Oct 2, 2023

However, the code still executes without a warning or error.

On 2.1.x I get two warnings, one for each line in the OP:

FutureWarning: The default fill_method='pad' in DataFrame.pct_change is deprecated and will be removed in a future version. Call ffill before calling pct_change to retain current behavior and silence this warning.
a.pct_change()

FutureWarning: The 'fill_method' and 'limit' keywords in DataFrame.pct_change are deprecated and will be removed in a future version. Call ffill before calling pct_change instead.
a.pct_change(fill_method=None)

Are you sure you're running with pandas 2.1.1 when you don't see warnings?

@rhshadrach rhshadrach added the Needs Info Clarification about behavior needed to assess issue label Oct 2, 2023
@MMCMA
Copy link
Author

MMCMA commented Oct 11, 2023

It seems the warning got supressed due to some test settings in my local enviroment - I see the warning on our servers, sorry for the confusion. However, I would still expect an error according to the documentation.

Generally, I am still a bit confused. The argument will eventually be deprecated but currently I still need to (as of 2.1.1) use it with an explizit a.pct_change(fill_method=None) to mimick the future default behaviour and to avoid NA fills. In the future, I will need to eventually drop fill_method=None and no NA fills will be applied, unless I use ffill() prior to calling .pct_change() . In short, currently I cannot write code that will work once fill_method is dropped for good and that does not ffill() the data.

@rhshadrach
Copy link
Member

Thanks @MMCMA - I believe your 2nd paragraph makes this a duplicate of the discussion here:

#53491 (comment)

Let me know if this is not the case and we can reopen!

@rhshadrach rhshadrach added Warnings Warnings that appear or should be added to pandas Transformations e.g. cumsum, diff, rank and removed Needs Info Clarification about behavior needed to assess issue Needs Triage Issue that has not been reviewed by a pandas team member labels Oct 12, 2023
@MMCMA
Copy link
Author

MMCMA commented Oct 13, 2023

Thanks @rhshadrach - yes you can close it - the documentation could still be a bit more clear here in my opinion but I guess it might be resolved within the duplicate issue.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Transformations e.g. cumsum, diff, rank Warnings Warnings that appear or should be added to pandas
Projects
None yet
Development

No branches or pull requests

2 participants