Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
DEPR: passing mixed offsets with utc=False into to_datetime #54014
DEPR: passing mixed offsets with utc=False into to_datetime #54014
Changes from all commits
bcbe2ac
ba99e93
441e17f
5e568bd
2aa5c10
8197005
ca4b214
a0e970a
156ea8a
643d3d6
61d4deb
f2bbedf
5c4904a
f5e5e1e
88f3845
3d74972
88ed6c1
180ccce
6ecd997
dc7c54d
b5bbd2b
1220130
0549e6d
b01c3ef
04ef036
c42f143
5e5adc6
3aa091f
acee8de
b7a2207
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The message here says that this will raise a warning in the future. But is that indeed the intent, or should that be "error" instead?
(my understanding of the discussion was that it would error in the future)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you're right, thanks - @natmokval fancy addressing this in a separate PR?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sure, I replaced "warning" with "error" in warning message and made a new PR
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
i think this might break if we ever had a tzaware datetime object with a dateutil/pytz tzinfo in the input array bc those tzinfos are not hashable
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks fine:
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
woops, its dateutil tzs that aren't hashable. though your example still seems to work with one, so never mind
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
there's a couple more usages of
to_datetime
in this function, I think we should ideally silence them tooThere was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you, I added
catch_warnings()
in two other places where we callto_datetime
in this function.I get failures in ci and I’m not sure if they are related to my changes. Locally all tests pass.
e.g the tests below failed in ci, but passed locally:
we don't even call this function (
converter()
) while running these tests.