API: ensure IntervalIndex.left/right are 64bit if numeric #50130
Conversation
This file contains 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
topper-123
force-pushed
the
interval._aybe_convert_i8
branch
from
a009f43
to
7e1fbdf
Compare
topper-123
force-pushed
the
interval._aybe_convert_i8
branch
from
7e1fbdf
to
5835d00
Compare
phofl
reviewed
Dec 11, 2022
pandas/core/indexes/interval.py
Outdated
| def _maybe_convert_numeric_to_64bit(self, idx: Index) -> Index: | ||
| # IntervalTree only supports 64 bit numpy array | ||
| dtype = idx.dtype | ||
| if np.issubclass_(idx.dtype.type, np.number): |
Not for me personally, it was more of a question I had when I wrote the code, and we now will support non-64-bit indexes. IMO this can come later, if someone sees a need for it. |
|
Sounds good to me. What we have to keep in mind a bit: If we compile IntervalTree for all possible dtypes, this will further increase the size of pandas for end users. So as long as there is no use case, I would keep as is. But open to revisit if there is a situation where this would be helpful |
|
Yeah, I agree. I’ve updated the PR. |
phofl
approved these changes
Dec 11, 2022
|
Thx @topper-123 |
This was referenced Dec 11, 2022
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Extracted part for #49560, slightly altered.
IntervalIndexdepends onIntervalTreewhich only accepts 64-bit int/uint/floats. This ensures that numpy numeric arrays inIntervalTreewill always be 64-bit.A question is if we should have non-64bit
IntervalTrees. I've considered that that is maybe a different question than #49650, and I could write an issue about it and someone could take it in a follow-up, possibly before 2.0?This PR changes nothing functionality-wise, but is needed when
Indexcan have non-64bit indexes after #49650.