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: Timestamp.replace handle out-of-pydatetime range #50348

Merged
merged 15 commits into from
Feb 11, 2023

Conversation

jbrockmendel
Copy link
Member

  • closes #xxxx (Replace xxxx with the GitHub issue number)
  • Tests added and passed if fixing a bug or adding a new feature
  • All code checks passed.
  • Added type annotations to new arguments/methods/functions.
  • Added an entry in the latest doc/source/whatsnew/vX.X.X.rst file if fixing a bug or adding a new feature.

Copy link
Member

@phofl phofl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks like the numpy dev build is failing

@mroeschke mroeschke added the Timestamp pd.Timestamp and associated methods label Dec 27, 2022
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

gh numbers missing, but other than that looks good, nice!

pandas/tests/tseries/offsets/test_year.py Show resolved Hide resolved
Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wait sorry, the numpy-dev failure seems legit

2022-12-26T21:13:19.3570839Z _______________________ test_add_out_of_pydatetime_range _______________________
2022-12-26T21:13:19.3571781Z [gw1] linux -- Python 3.10.8 /home/runner/micromamba/envs/test/bin/python
2022-12-26T21:13:19.3572026Z 
2022-12-26T21:13:19.3572174Z     def test_add_out_of_pydatetime_range():
2022-12-26T21:13:19.3572564Z         # don't raise in Timestamp.replace
2022-12-26T21:13:19.3572976Z         ts = Timestamp(np.datetime64("-20000-12-31"))
2022-12-26T21:13:19.3573268Z         off = YearEnd()
2022-12-26T21:13:19.3573482Z     
2022-12-26T21:13:19.3573705Z         result = ts + off
2022-12-26T21:13:19.3574104Z         expected = Timestamp(np.datetime64("-19999-12-31"))
2022-12-26T21:13:19.3574417Z >       assert result == expected
2022-12-26T21:13:19.3574932Z E       AssertionError: assert Timestamp('1973-12-31 00:00:00') == Timestamp('-19999-12-31 00:00:00')

@MarcoGorelli MarcoGorelli added the Non-Nano datetime64/timedelta64 with non-nanosecond resolution label Jan 4, 2023
@jbrockmendel
Copy link
Member Author

yah i havent been able to reproduce the npdev failure. will add GH refs in the next commit once i figure that out

@jbrockmendel
Copy link
Member Author

@seberg any thoughts on reproducing this locally?

@seberg
Copy link
Contributor

seberg commented Jan 7, 2023

Hmmm, unfortunately, can't repro on M1 and I can't think of anything that may have changed in NumPy.

@MarcoGorelli
Copy link
Member

ooh, looks like this fixes #51291, nice!

Copy link
Member

@MarcoGorelli MarcoGorelli left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me

Got a comment about simplifying something, but not important / blocking

pandas/_libs/tslibs/timestamps.pyx Outdated Show resolved Hide resolved
@MarcoGorelli
Copy link
Member

@pytest.mark.xfail(is_numpy_dev, reason="result year is 1973, unclear why")

btw I can't reproduce this locally with numpy-dev, the correct answer shows for me

Co-authored-by: Marco Edward Gorelli <33491632+MarcoGorelli@users.noreply.github.com>
@MarcoGorelli MarcoGorelli added this to the 2.0 milestone Feb 11, 2023
@MarcoGorelli
Copy link
Member

let's ship this - thanks @jbrockmendel

@MarcoGorelli MarcoGorelli merged commit 4b65e2f into pandas-dev:main Feb 11, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Non-Nano datetime64/timedelta64 with non-nanosecond resolution Timestamp pd.Timestamp and associated methods
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants