-
-
Notifications
You must be signed in to change notification settings - Fork 18.4k
BUG: isin incorrectly casting ints to datetimes #37528
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
Merged
Merged
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
6f52f7d
BUG: isin incorrectly casting ints to datetimes
jbrockmendel 023b437
GH ref
jbrockmendel 565f9a3
Merge branch 'master' of https://github.com/pandas-dev/pandas into bu…
jbrockmendel 0c73202
Merge branch 'master' of https://github.com/pandas-dev/pandas into bu…
jbrockmendel 6a5e1d7
Merge branch 'master' of https://github.com/pandas-dev/pandas into bu…
jbrockmendel 7b4199d
Merge branch 'master' of https://github.com/pandas-dev/pandas into bu…
jbrockmendel 9758b02
Merge branch 'master' of https://github.com/pandas-dev/pandas into bu…
jbrockmendel ca75491
add asvs
jbrockmendel File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
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
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
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
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.
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.
are you sure?
ensure_data and below does this
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 way we call ensure_data ends up casting dt64 to int64
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.
why can't you fix it in _ensure_data then, I am not averse to dispatching to the EA itself, which is what we are already doing for Categorical, or is that the plan. If that is the plan, then I would think you could actually remove all off this and simply dispatch (e.g. most of this logic then would actually be handled by PandasArray or the EA as appropriate).
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 are a bunch of GH issues about isin (added a new label yesterday), many of which are about unwanted casting. I haven't looked at them all closely tet.
Even if we fix that (which yes, i'll take a look, but probably in a separate PR), we still need to dispatch to the
(Period|Timedelta|Datetime)Index
to get correct casting of theother
(assuming we wantalgos.isin(dtlike, other)
to matchdtlike.isin(other)
, which I for one do)ATM Categorical has
isin
but thats it. #20617 suggests adding isin to EA, but thats definitely outside the scope of this PR.