-
-
Notifications
You must be signed in to change notification settings - Fork 18.8k
BUG: using dtype='int64' argument of Series causes ValueError: values cannot be losslessly cast to int64 for integer strings #45017
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
Changes from 7 commits
7952825
5d0362e
01af816
730d5a9
204c3c9
814e5ff
a76a34a
8603236
40b3872
13ca6df
c4840da
55410de
eed91cb
ae6ea47
1247f0e
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 |
---|---|---|
|
@@ -1810,6 +1810,19 @@ def test_constructor_bool_dtype_missing_values(self): | |
expected = Series(True, index=[0], dtype="bool") | ||
tm.assert_series_equal(result, expected) | ||
|
||
@pytest.mark.parametrize("int_dtype", ["int64"]) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use this fixture instead: any_int_dtype There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
def test_constructor_int64_dtype(self, int_dtype): | ||
# GH-44923 | ||
result = Series(["-1", "0", "1", "2"], dtype=int_dtype) | ||
expected = Series([-1, 0, 1, 2]) | ||
tm.assert_series_equal(result, expected) | ||
|
||
def test_constructor_float64_dtype(self): | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. use any_float_dtype There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
# GH-44923 | ||
result = Series(["0", "1", "2"], dtype="float64") | ||
expected = Series([0.0, 1.0, 2.0]) | ||
tm.assert_series_equal(result, expected) | ||
|
||
@pytest.mark.filterwarnings( | ||
"ignore:elementwise comparison failed:DeprecationWarning" | ||
) | ||
|
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 this always holds. The condition we're interested in is whether the casting was lossy.
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.
Agreed, but this test currently passes the extra test added.
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 the added tests pass w/o this change?
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.
Yes they do.
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.
Right, because this condition always holds.
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.
But before adding this check the testcase in the issue was not passing.
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.
That's a reason to add some check for this, but this particular check is not the right check. ATM this is equivalent to (but slower than)
if True: