Skip to content

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.

Sign up for GitHub

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

Closed
wants to merge 15 commits into from
Closed

BUG: using dtype='int64' argument of Series causes ValueError: values cannot be losslessly cast to int64 for integer strings #45017

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
7952825
Valueerror
shubham11941140 Dec 22, 2021
5d0362e
precommit
shubham11941140 Dec 22, 2021
01af816
Merge branch 'master' of https://github.com/pandas-dev/pandas into b7
shubham11941140 Dec 23, 2021
730d5a9
Optimization
shubham11941140 Dec 23, 2021
204c3c9
infer_dtype added
shubham11941140 Dec 23, 2021
814e5ff
precommit
shubham11941140 Dec 23, 2021
a76a34a
Parametrised
shubham11941140 Dec 27, 2021
8603236
changed to any_dtype
shubham11941140 Dec 31, 2021
40b3872
Added xfail
shubham11941140 Jan 1, 2022
13ca6df
precommit
shubham11941140 Jan 1, 2022
c4840da
Removed parametrise
shubham11941140 Jan 4, 2022
55410de
precommit fix
shubham11941140 Jan 4, 2022
eed91cb
removed obvious line
shubham11941140 Jan 17, 2022
ae6ea47
Checking string dtype directly
shubham11941140 Feb 11, 2022
1247f0e
Merge branch 'main' into b7
shubham11941140 Mar 7, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions pandas/core/dtypes/cast.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2096,6 +2096,9 @@ def maybe_cast_to_integer_array(
)
return casted

if lib.infer_dtype(casted) == "integer":
Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Contributor

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?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes they do.

Copy link
Member

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.

Copy link
Contributor Author

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.

Copy link
Member

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.

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:

return casted

# No known cases that get here, but raising explicitly to cover our bases.
raise ValueError(f"values cannot be losslessly cast to {dtype}")

Expand Down
14 changes: 14 additions & 0 deletions pandas/tests/series/test_constructors.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1810,6 +1810,20 @@ def test_constructor_bool_dtype_missing_values(self):
expected = Series(True, index=[0], dtype="bool")
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize("any_int_dtype", ["int64"])
def test_constructor_int64_dtype(self, any_int_dtype):
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, pls just use the fixture itself, e.g. no parameterize

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is causing Assertion Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

image

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous code segment is leading to this issue, if we have only int64 there is no issue.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you need to match the expected value as well

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback I think I have covered everything?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@shubham11941140 you are not using the fixtures pls do so

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

just remove the paramterize completely

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

uint -> uint8, uint16, uint32, uint64 are failing due to internal code implementation. Do i fix this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@jreback removed parametrization, now it should be ready.

# GH-44923
result = Series(["-1", "0", "1", "2"], dtype=any_int_dtype)
expected = Series([-1, 0, 1, 2])
tm.assert_series_equal(result, expected)

@pytest.mark.parametrize("any_float_dtype", ["float64"])
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

same

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It is the same as the previous image.

def test_constructor_float64_dtype(self, any_float_dtype):
# GH-44923
result = Series(["0", "1", "2"], dtype=any_float_dtype)
expected = Series([0.0, 1.0, 2.0])
tm.assert_series_equal(result, expected)

@pytest.mark.filterwarnings(
"ignore:elementwise comparison failed:DeprecationWarning"
)
Expand Down