Skip to content

BUG to_timedelta was raising ValueError w pd.NA #53022

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 7 commits into from
May 3, 2023
Merged

BUG to_timedelta was raising ValueError w pd.NA #53022

merged 7 commits into from
May 3, 2023

Conversation

DeaMariaLeon
Copy link
Member

@DeaMariaLeon DeaMariaLeon requested a review from MarcoGorelli May 1, 2023 17:50
@DeaMariaLeon
Copy link
Member Author

DeaMariaLeon commented May 2, 2023 •
edited
Loading

I think this is partially fixing #40566 - the first 3 cases on the issue description are no longer failing.

Edit:
Now I can't see which test fails.

MarcoGorelli
MarcoGorelli suggested changes May 2, 2023
View reviewed changes
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.

nice! looks like this is close, just have a minor comment and a question

pandas/tests/series/methods/test_astype.py
@@ -471,7 +471,7 @@ class TestAstypeString:
def test_astype_string_to_extension_dtype_roundtrip(
self, data, dtype, request, nullable_string_dtype
):
if dtype == "boolean" or (dtype == "timedelta64[ns]" and NaT in data):
Copy link
Member

Choose a reason for hiding this comment

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

just for my understanding, why did this one change?

Copy link
Member

Choose a reason for hiding this comment

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

ah it's because

ser.astype(nullable_string_dtype)

introduces '<NA>', and then .astype('timedelta64[ns]') couldn't handle that (but it can after this PR!)

@mroeschke mroeschke added Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Timedelta Timedelta data type labels May 2, 2023
@MarcoGorelli MarcoGorelli changed the title BUG timedelta was raising ValueError w pd.NA BUG to_timedelta was raising ValueError w pd.NA May 3, 2023
MarcoGorelli
MarcoGorelli approved these changes May 3, 2023
View reviewed changes
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, thanks @DeaMariaLeon !

Leaving open til tomorrow in case anyone else has comments, then will merge

pandas/tests/series/methods/test_astype.py
@@ -471,7 +471,7 @@ class TestAstypeString:
def test_astype_string_to_extension_dtype_roundtrip(
self, data, dtype, request, nullable_string_dtype
):
if dtype == "boolean" or (dtype == "timedelta64[ns]" and NaT in data):
Copy link
Member

Choose a reason for hiding this comment

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

ah it's because

ser.astype(nullable_string_dtype)

introduces '<NA>', and then .astype('timedelta64[ns]') couldn't handle that (but it can after this PR!)

@MarcoGorelli MarcoGorelli added this to the 2.1 milestone May 3, 2023
@DeaMariaLeon
Copy link
Member Author

Thanks @MarcoGorelli for reviewing & merging. I even got a cat! :-)

@mroeschke mroeschke merged commit f8a503d into pandas-dev:main May 3, 2023
@mroeschke
Copy link
Member

Thanks @DeaMariaLeon

@DeaMariaLeon
Copy link
Member Author

Thank you for merging @mroeschke

topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 7, 2023
@DeaMariaLeon @topper-123
BUG to_timedelta was raising ValueError w pd.NA (pandas-dev#53022)
a43d97d 
* BUG fixing timedelta

* BUG timedelta raising w pd.NA

* BUG removed not needed fail on test .. ?

* BUG Parametrised test
@DeaMariaLeon DeaMariaLeon deleted the timedelta branch May 8, 2023 14:45
Rylie-W pushed a commit to Rylie-W/pandas that referenced this pull request May 19, 2023
@DeaMariaLeon
BUG to_timedelta was raising ValueError w pd.NA (pandas-dev#53022)
c126ddd 
* BUG fixing timedelta

* BUG timedelta raising w pd.NA

* BUG removed not needed fail on test .. ?

* BUG Parametrised test
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
@DeaMariaLeon
BUG to_timedelta was raising ValueError w pd.NA (pandas-dev#53022)
89b7854 
* BUG fixing timedelta

* BUG timedelta raising w pd.NA

* BUG removed not needed fail on test .. ?

* BUG Parametrised test
@jbrockmendel
Copy link
Member

i think this fixed #38509 too!

@jbrockmendel jbrockmendel mentioned this pull request Jul 27, 2023
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mroeschke mroeschke mroeschke approved these changes

@MarcoGorelli MarcoGorelli MarcoGorelli approved these changes

Assignees
No one assigned
Labels
Missing-data np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate Timedelta Timedelta data type
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.
4 participants
@DeaMariaLeon @mroeschke @jbrockmendel @MarcoGorelli