-
-
Notifications
You must be signed in to change notification settings - Fork 19.1k
BUG: Raise TypeError for mismatched signed/unsigned dtypes in IntervalIndex.from_arrays #62376
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
base: main
Are you sure you want to change the base?
Changes from 3 commits
736ae02
439efbe
ed887d9
f7433c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -536,6 +536,21 @@ def from_arrays( | |
left = _maybe_convert_platform_interval(left) | ||
right = _maybe_convert_platform_interval(right) | ||
|
||
# Check for mismatched signed/unsigned integer dtypes | ||
left_dtype = getattr(left, "dtype", None) | ||
right_dtype = getattr(right, "dtype", None) | ||
|
||
if ( | ||
left_dtype is not None | ||
and right_dtype is not None | ||
and left_dtype.kind in "iu" | ||
and right_dtype.kind in "iu" | ||
and left_dtype.kind != right_dtype.kind | ||
): | ||
raise TypeError( | ||
f"Left and right arrays must have matching signedness. " | ||
f"Got {left_dtype} and {right_dtype}." | ||
) | ||
|
||
left, right, dtype = cls._ensure_simple_new_inputs( | ||
left, | ||
right, | ||
|
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 putting this after the _maybe_convert_platform_interval calls would be more robust. e.g. if one is a list and the other is uint64?
Also is it just int vs uint we care about, or also e.g. int32 vs 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.
We are checking int vs unit .
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 reason not to move this to after the _maybe_convert_platform_interval calls?
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.
no, i have updated, also added whatsnew note, please let me know if this needs improvement.