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
BUG: Timestamp == date match stdlib #36131
BUG: Timestamp == date match stdlib #36131
Changes from 20 commits
4c5eddd
c632c9f
9e64be3
42649fb
47121dd
1decb3e
57c5dd3
a358463
ffa7ad7
e5e98d4
408db5a
9b1b3f3
a6d4228
5517a68
5ce9683
f43e920
1bffa44
8c6012e
ec827a0
fbb1576
efae3ad
e53dc16
e504852
5b34905
f0060e3
f97848e
41ae9cd
f0add43
da0888c
27c8005
8036c99
a8fed20
198d9e0
67429a1
3513f3b
6df2513
dbb3086
87ff44a
c97a1e7
67b3fb2
da9426c
13f170b
ba18c96
de69fb4
d412ac0
cf85a4f
a1e32d7
938ad02
06589ca
4cd919c
947daac
5196b1e
4d7aa41
a307a0c
a92409d
164e2b4
a321fdd
0cb6019
3557c7a
af49cb5
7a423f4
677760f
b8c67e9
fbfdaff
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.
Is there a way to avoid this warning in this case? (it's not something a user can do something about?)
And will the behaviour change here after the deprecationg has been enforced?
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.
any thoughts here @jbrockmendel
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.
just the warning not to use date objects, or removing the Categorical behavior of passing convert_dates=True to maybe_infer_to_datetimelike
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.
this should not be warning at the top level
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.
In this the user/test is passing date objects, and gets a warning that behavior with date objects is going to change.
I can add a test in which the user-facing behavior is going to change (and is currently wrong)
But after this deprecation is enforced, we're going to end up with
which btw is analogous to what we'd get today if we swapped tup and tup2 above:
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.
we are exposing this to the user?
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.
any idea why this happens?
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.
when it tries to make a MultiIndex out of the A and B columns, it goes through Categorical, which passes convert_dates=True to maybe_infer_to_datetimelike,
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.
this should not be warnign at the top level
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.
we could finagle it so that we dont pass convert_dates=True to maybe_infer_to_datetimelike in this case, but that gets pretty complicated, especially compared to just warning the user that date objects are finicky
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 like a while back you were in favor of telling people not to pass dates to the MultiIndex constructors.