API/BUG: infer_dtype_from_scalar with non-nano by jbrockmendel · Pull Request #52212 · pandas-dev/pandas (original) (raw)
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 andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Conversation14 Commits14 Checks0 Files changed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.Learn more about bidirectional Unicode characters
[ Show hidden characters]({{ revealButtonHref }})
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.
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
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
Comment on lines +1358 to +1361
| mark = pytest.mark.xfail( |
|---|
| reason="Construction from dict goes through " |
| "maybe_convert_objects which casts to nano" |
| ) |
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?
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)
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
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?
🤔 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'.
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?
topper-123 pushed a commit to topper-123/pandas that referenced this pull request
API/BUG: infer_dtype_from_scalar with non-nano
update test
xfail on 32bit
fix xfail condition
whatsnew
xfail on windows
Daquisu pushed a commit to Daquisu/pandas that referenced this pull request
API/BUG: infer_dtype_from_scalar with non-nano
update test
xfail on 32bit
fix xfail condition
whatsnew
xfail on windows
Labels
datetime64/timedelta64 with non-nanosecond resolution