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.

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

API/BUG: infer_dtype_from_scalar with non-nano #52212

Merged
merged 14 commits into from
May 18, 2023
Merged

API/BUG: infer_dtype_from_scalar with non-nano #52212

merged 14 commits into from
May 18, 2023

Conversation

jbrockmendel
Copy link
Member

Needs whatsnew and targeted tests, opening to see if there are opinions about doing this as a bugfix or waiting to do it as an API change in 3.0

@github-actions
Copy link
Contributor

github-actions bot commented May 4, 2023

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

@github-actions github-actions bot added the Stale label May 4, 2023
@jbrockmendel
Copy link
Member Author

@MarcoGorelli thoughts on if/how this is worth pursuing?

@MarcoGorelli
Copy link
Member

sorry, missed this. I'd have thought this was fine as a bug fix - I'm at jupytercon next week though so may be a bit slow to review, but will take a look when I get a chance

@MarcoGorelli MarcoGorelli self-requested a review May 11, 2023 10:24
MarcoGorelli
MarcoGorelli requested changes May 15, 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! just got some minor comments

pandas/tests/series/test_constructors.py
Comment on lines +1358 to +1361
mark = pytest.mark.xfail(
reason="Construction from dict goes through "
"maybe_convert_objects which casts to nano"
)
Copy link
Member

Choose a reason for hiding this comment

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

this risks staying here forever, could you add strict=True please, so that when it's fixed we remember to remove this?

Copy link
Member Author

Choose a reason for hiding this comment

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

is strict=True no longer the default? if so, will update. (also if so: yikes)

Copy link
Member

Choose a reason for hiding this comment

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

nvm, ignore me, it's configured to be the default

pandas/pyproject.toml

Line 441 in 7304396

xfail_strict = true

pandas/tests/frame/test_constructors.py Outdated Show resolved Hide resolved
pandas/tests/frame/test_constructors.py Outdated Show resolved Hide resolved
MarcoGorelli
MarcoGorelli approved these changes May 16, 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.

Changes look good to me, I think it'd be OK to get this into 2.1 as it'd be a bug fix

do you want to add a whatsnew note?

@MarcoGorelli
Copy link
Member

🤔 not sure if this might be related

FAILED pandas/tests/frame/methods/test_reindex.py::TestDataFrameSelectReindex::test_reindex_tzaware_fill_value - ValueError: Unexpected value for 'dtype': 'int32'. Must be 'datetime64[s]', 'datetime64[ms]', 'datetime64[us]', 'datetime64[ns]' or DatetimeTZDtype'.

MarcoGorelli
MarcoGorelli approved these changes May 18, 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

@mroeschke any comments, or good to go?

@mroeschke mroeschke added this to the 2.1 milestone May 18, 2023
@mroeschke mroeschke added Non-Nano datetime64/timedelta64 with non-nanosecond resolution and removed Stale labels May 18, 2023
@mroeschke mroeschke merged commit a2bb939 into pandas-dev:main May 18, 2023
@mroeschke
Copy link
Member

Thanks @jbrockmendel

@jbrockmendel jbrockmendel deleted the bugs-nano branch May 18, 2023 17:16
topper-123 pushed a commit to topper-123/pandas that referenced this pull request May 22, 2023
@jbrockmendel @topper-123
API/BUG: infer_dtype_from_scalar with non-nano (pandas-dev#52212)
bdf50a2 
* API/BUG: infer_dtype_from_scalar with non-nano

* update test

* xfail on 32bit

* fix xfail condition

* whatsnew

* xfail on windows
@jtilly jtilly mentioned this pull request May 23, 2023
Closed
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request Jul 8, 2023
@jbrockmendel
API/BUG: infer_dtype_from_scalar with non-nano (pandas-dev#52212)
b04a889 
* API/BUG: infer_dtype_from_scalar with non-nano

* update test

* xfail on 32bit

* fix xfail condition

* whatsnew

* xfail on windows
@IzerOnadimQC IzerOnadimQC mentioned this pull request Oct 16, 2023
Closed
3 tasks
@iainrussell iainrussell mentioned this pull request Oct 18, 2023
Closed
@IzerOnadimQC IzerOnadimQC mentioned this pull request Oct 29, 2023
Merged
1 task
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
Non-Nano datetime64/timedelta64 with non-nanosecond resolution
Projects
None yet
Milestone
2.1
Development

Successfully merging this pull request may close these issues.

BUG: infer_dtype_from_scalar infers nanosecond dtype for second input
3 participants
@jbrockmendel @MarcoGorelli @mroeschke