-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
BUG: Series constructor overflowing for UInt64 #50757
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
Conversation
Yikes this is mean, it's not failing on Mac :) |
) | ||
tm.assert_series_equal(result, expected) | ||
|
||
def test_series_constructor_overflow_int_with_nan(self): |
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.
should this be "uint" instead of "int"?
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.
Thx, yes
@@ -2007,6 +2008,50 @@ def test_series_constructor_ea_int_from_string_bool(self): | |||
with pytest.raises(ValueError, match="invalid literal"): | |||
Series(["True", "False", "True", pd.NA], dtype="Int64") | |||
|
|||
@pytest.mark.parametrize("val", [1, 1.0]) | |||
def test_series_constructor_overflow_uint_ea(self, val): |
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.
do these tests go through meaningfully different paths? if not id suggest combining them. not a big deal
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.
Expected is annoying to define in one df, so would prefer keeping it separate to make it easier to read
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.
LGTM
doc/source/whatsnew/vX.X.X.rst
file if fixing a bug or adding a new feature.Not sure if there is a way to avoid loss of precision without using object in the mixed case. This does not impact the regular case without overflow risks