Skip to content

BUG: Inconsistent behavior for df.replace over pd.Period columns #34871

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

Closed
2 of 3 tasks
justinessert opened this issue Jun 19, 2020 · 6 comments · Fixed by #36867
Closed
2 of 3 tasks

BUG: Inconsistent behavior for df.replace over pd.Period columns #34871

justinessert opened this issue Jun 19, 2020 · 6 comments · Fixed by #36867
Assignees
Labels
good first issue Needs Tests Unit test(s) needed to prevent regressions Period Period data type replace replace method
Milestone

Comments

@justinessert
Copy link
Contributor

  • 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.


Code Sample, a copy-pastable example

import pandas as pd

df = pd.DataFrame({
    'Int': [1, 2, 3],
    'Flo': [1.0, 6.0, 1.0],
    'Per': [pd.Period('2020-01')]*3,
    'Tim': [pd.Timestamp('2020-01')]*3
})

# TypeError: 'value' should be a 'Period', 'NaT', or array of those. Got 'float' instead.
df.replace(1.0, 0.0)

# Works
df[['Int', 'Flo', 'Tim']].replace(1.0, 0.0)

# Works
df['Per'].replace(1.0, pd.Period('2020-02'))

Problem description

df.replace(1.0, 0.0) fails if any columns of df are Period types. The error is saying that 0.0 is an invalid value to put in a Pandas Period column. This should never be an issue though, 1.0 will never exist in a Pandas Period column, so nothing will ever be replaced for 0.0.

Expected Output

The expected behavior would be consistent with the df[['Int', 'Flo', 'Tim']].replace(1.0, 0.0) in the example above. This does work, the 'Flo' column has all of it's 1.0s replaced with 0.0s and the 'Int' and 'Tim' columns (which are not Float types) remain unaffected. I would expect that a Pandas Period column would also be skipped in the same way.

Output of pd.show_versions()

INSTALLED VERSIONS

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

pandas : 1.0.4
numpy : 1.18.5
pytz : 2020.1
dateutil : 2.8.0
pip : 20.1.1
setuptools : 47.3.0.post20200616
Cython : None
pytest : 4.4.1
hypothesis : None
sphinx : 3.1.1
blosc : None
feather : None
xlsxwriter : 1.2.9
lxml.etree : 4.5.1
html5lib : None
pymysql : None
psycopg2 : None
jinja2 : 2.11.2
IPython : 7.15.0
pandas_datareader: None
bs4 : None
bottleneck : None
fastparquet : None
gcsfs : None
lxml.etree : 4.5.1
matplotlib : 3.2.1
numexpr : None
odfpy : None
openpyxl : None
pandas_gbq : None
pyarrow : None
pytables : None
pytest : 4.4.1
pyxlsb : None
s3fs : None
scipy : 1.4.1
sqlalchemy : None
tables : None
tabulate : None
xarray : None
xlrd : 1.2.0
xlwt : None
xlsxwriter : 1.2.9
numba : None

@justinessert justinessert added Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Jun 19, 2020
@dsaxton
Copy link
Member

dsaxton commented Jun 19, 2020

This looks to be fixed on master (but could warrant a test if there isn't one already). Interesting though that the 1 in the first column appears not to be replaced.

>>> import pandas as pd
>>> 
>>> df = pd.DataFrame({
...     'Int': [1, 2, 3],
...     'Flo': [1.0, 6.0, 1.0],
...     'Per': [pd.Period('2020-01')]*3,
...     'Tim': [pd.Timestamp('2020-01')]*3
... })
>>> print(df.replace(1.0, 0.0))
   Int  Flo      Per        Tim
0    1  0.0  2020-01 2020-01-01
1    2  6.0  2020-01 2020-01-01
2    3  0.0  2020-01 2020-01-01
>>> print(pd.__version__)
1.1.0.dev0+1900.g80ba4c429

@dsaxton dsaxton added Needs Tests Unit test(s) needed to prevent regressions Period Period data type and removed Bug Needs Triage Issue that has not been reviewed by a pandas team member labels Jun 19, 2020
@justinessert
Copy link
Contributor Author

I just realized that Pandas 1.0.5 was released, I was running this on version 1.0.4. I believe everything is fixed now!

@nwweber
Copy link
Contributor

nwweber commented Jun 20, 2020

take

@nwweber
Copy link
Contributor

nwweber commented Jun 20, 2020

Hi all,

I can confirm that this failed in 1.0.4 and indeed does not raise an exception anymore in 1.10.

However, the new behavior in 1.10 is still not what I would expect it to be. Concretely, calling df.replace(1.0, 0.0) on a dataframe with a Period column now has the following effects:

  • the values in a Period column are now left unchanged
  • but the dtype of that same column is changed from Period to Object

I think a user would be surprised by this change in dtype (I certainly am). This also makes tm.assert_frame_equal() fail when trying to write a test for this.

For now I have created a tiny regression test that makes sure that calling df.replace() does not raise an exception anymore, see this pull request: #34904

I think this issue should stay open and df.replace() should get changed to return a Period column in the scenario described. That's a bit beyond my current insights into the Pandas code, it'd be great if someone else could have a look at that.

Minimal code to reproduce (adapted from original submitter):

df = pd.DataFrame({"Per": [pd.Period("2020-01")] * 3})
new_df = df.replace(1.0, 0.0)

# this is 'Period'
df.Per.dtype
# this is 'Object' but should also be 'Period'
new_df.Per.dtype

@nwweber
Copy link
Contributor

nwweber commented Jun 26, 2020

Unit test for desired behavior implemented here: #34904

Will have a look at underlying issue causing modification of dtype from period to object

@tsibley
Copy link

tsibley commented Jul 30, 2020

Will have a look at underlying issue causing modification of dtype from period to object

I believe this is the same problem described in #35268.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Needs Tests Unit test(s) needed to prevent regressions Period Period data type replace replace method
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants