Skip to content

BUG: Adds missing raises for numpy.timedelta64[M/Y] in pandas.Timedelta #53421

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

Merged
merged 27 commits into from
Jun 20, 2023

Conversation

mcgeestocks
Copy link
Contributor

@mcgeestocks mcgeestocks commented May 27, 2023

@mcgeestocks
Copy link
Contributor Author

  • Uploading this draft to make sure I'm in the right direction.
  • Will add tests into if thistest_timedeltas.py if appropriate.
  • Let me know if I need to improve the raise message @mroeschke.

@mcgeestocks
Copy link
Contributor Author

Looking at the 3 failed tests:

  1. test_constructors.py::test_construction_out_of_bounds_td64ns
  2. test_constructors.py::test_construction_out_of_bounds_td64s
  3. test_lib.py::TestMisc::test_fast_multiget_timedelta_resos

My assumption is that the correct approach would be to alter tests to now expect a value error to be thrown but I'm a little unsure if the is the right mentality.

I'm a little unsure of the best approach though, any guidance would appreciated.

@mcgeestocks
Copy link
Contributor Author

Went ahead and updated the test to reflect the newly added raise condition.

@mcgeestocks mcgeestocks marked this pull request as ready for review May 30, 2023 13:43
@mroeschke mroeschke added Error Reporting Incorrect or improved errors from pandas Non-Nano datetime64/timedelta64 with non-nanosecond resolution labels May 30, 2023
@mcgeestocks mcgeestocks marked this pull request as draft May 31, 2023 16:40
@mcgeestocks mcgeestocks marked this pull request as ready for review May 31, 2023 19:07
@mcgeestocks mcgeestocks marked this pull request as draft June 2, 2023 19:11
@mcgeestocks mcgeestocks marked this pull request as ready for review June 5, 2023 22:53
@mcgeestocks mcgeestocks requested a review from MarcoGorelli as a code owner June 5, 2023 22:53
@mcgeestocks mcgeestocks requested a review from mroeschke June 8, 2023 16:26
@mcgeestocks mcgeestocks marked this pull request as draft June 8, 2023 22:07
@mcgeestocks mcgeestocks marked this pull request as ready for review June 9, 2023 20:54
@mcgeestocks mcgeestocks marked this pull request as draft June 9, 2023 20:54
@mcgeestocks mcgeestocks marked this pull request as ready for review June 10, 2023 00:08
@mcgeestocks
Copy link
Contributor Author

Finally ready... thanks for the patience @mroeschke

@mcgeestocks
Copy link
Contributor Author

Anything else needed here?

@mcgeestocks mcgeestocks requested a review from mroeschke June 19, 2023 20:31
@mroeschke mroeschke added this to the 2.1 milestone Jun 20, 2023
@mroeschke mroeschke merged commit 93aa939 into pandas-dev:main Jun 20, 2023
@mroeschke
Copy link
Member

Thanks @mcgeestocks

Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
…ta (pandas-dev#53421)

* adds raise for numpy Y and M conversion in Timedelta

* fix failing tests

* update raise lang

* alternative approach

* add missing periods

* switch to cimport

* add cython directive

* switch to inline cython directive

* remove unsupported td operation

* adds whatnew update

* remove whitespace

* adds test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Error Reporting Incorrect or improved errors from pandas Non-Nano datetime64/timedelta64 with non-nanosecond resolution
Projects
None yet
Development

Successfully merging this pull request may close these issues.

BUG: numpy.timedelta64[M] not properly raising in pandas.Timedelta in 2.0.0
2 participants