Skip to content

API: NumericIndex([1 2, 3]).dtype should be int64 on 32-bit systems #49815

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

Merged
merged 3 commits into from
Nov 22, 2022
Merged

API: NumericIndex([1 2, 3]).dtype should be int64 on 32-bit systems #49815

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
0e83211
API: NumericIndex([1 2, 3]).dtype should be int64 on 32-bit systems
topper-123 Nov 21, 2022
caf0c5a
cleanups
topper-123 Nov 21, 2022
0e7b532
cleanups II
topper-123 Nov 21, 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
8 changes: 0 additions & 8 deletions pandas/core/indexes/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4962,14 +4962,6 @@ def _raise_scalar_data_error(cls, data):
f"kind, {repr(data)} was passed"
)

@final
@classmethod
def _string_data_error(cls, data):
raise TypeError(
"String dtype not supported, you may need "
"to explicitly cast to a numeric type"
)

Copy link
Contributor Author

@topper-123 topper-123 Nov 21, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This was only used in NumericIndex.

def _validate_fill_value(self, value):
"""
Check if the value can be inserted into our array without casting,
Expand Down
14 changes: 9 additions & 5 deletions pandas/core/indexes/numeric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,7 @@
)
from pandas.core.dtypes.generic import ABCSeries

from pandas.core.construction import sanitize_array
from pandas.core.indexes.base import (
Index,
maybe_extract_name,
Expand Down Expand Up @@ -144,15 +145,17 @@ def _ensure_array(cls, data, dtype, copy: bool):
data = list(data)

orig = data
data = np.asarray(data, dtype=dtype)
if isinstance(data, (list, tuple)):
if len(data):
data = sanitize_array(data, index=None)
else:
data = np.array([], dtype=np.int64)

if dtype is None and data.dtype.kind == "f":
if cls is UInt64Index and (data >= 0).all():
# https://github.com/numpy/numpy/issues/19146
data = np.asarray(orig, dtype=np.uint64)

if issubclass(data.dtype.type, str):
cls._string_data_error(data)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removing this simplifies error handling by not special casing string data.

dtype = cls._ensure_dtype(dtype)

if copy or not is_dtype_equal(data.dtype, dtype):
Expand Down Expand Up @@ -198,7 +201,8 @@ def _ensure_dtype(cls, dtype: Dtype | None) -> np.dtype | None:
return cls._default_dtype

dtype = pandas_dtype(dtype)
assert isinstance(dtype, np.dtype)
if not isinstance(dtype, np.dtype):
raise TypeError(f"{dtype} not a numpy type")
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I strengthen this check here, though it will not be important in the end when everything is moved into Index


if cls._is_backward_compat_public_numeric_index:
# dtype for NumericIndex
Expand Down
29 changes: 14 additions & 15 deletions pandas/tests/indexes/numeric/test_numeric.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,10 @@ def check_coerce(self, a, b, is_float_index=True):
else:
self.check_is_index(b)

def test_constructor_from_list_no_dtype(self):
index = self._index_cls([1.5, 2.5, 3.5])
assert index.dtype == np.float64

def test_constructor(self, dtype):
index_cls = self._index_cls

Expand Down Expand Up @@ -115,17 +119,10 @@ def test_constructor_invalid(self):
with pytest.raises(TypeError, match=msg):
index_cls(0.0)

# 2021-02-1 we get ValueError in numpy 1.20, but not on all builds
msg = "|".join(
[
"String dtype not supported, you may need to explicitly cast ",
"could not convert string to float: 'a'",
]
)
with pytest.raises((TypeError, ValueError), match=msg):
msg = f"data is not compatible with {index_cls.__name__}"
with pytest.raises(ValueError, match=msg):
index_cls(["a", "b", 0.0])

msg = f"data is not compatible with {index_cls.__name__}"
with pytest.raises(ValueError, match=msg):
index_cls([Timestamp("20130101")])

Expand Down Expand Up @@ -327,18 +324,16 @@ def test_identical(self, simple_index, dtype):
assert not index.astype(dtype=object).identical(index.astype(dtype=dtype))

def test_cant_or_shouldnt_cast(self):
msg = (
"String dtype not supported, "
"you may need to explicitly cast to a numeric type"
)
msg = f"data is not compatible with {self._index_cls.__name__}"

# can't
data = ["foo", "bar", "baz"]
with pytest.raises(TypeError, match=msg):
with pytest.raises(ValueError, match=msg):
self._index_cls(data)

# shouldn't
data = ["0", "1", "2"]
with pytest.raises(TypeError, match=msg):
Copy link
Member

Choose a reason for hiding this comment

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

probably requires a whatsnew note

Copy link
Member

Choose a reason for hiding this comment

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

or could just keep the string_error_whatever to keep this tightly focused and avoid the API change

Copy link
Contributor Author

@topper-123 topper-123 Nov 22, 2022 •
edited
Loading

Choose a reason for hiding this comment

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

This error can in pandas <2.0 only be hit by instantiating Int64Index et al. directly, because Index will give a object dtype to this. Int64Index will be removed in pandas 2.0, so IMO a whatsnew notes should not be added?

Doesn't hit this either it we combine string data with numeric dtype:

>>> import pandas as pd
>>> pd.Index(["1", "2"], dtype="int64")
ValueError: Trying to coerce float values to integers  # different code path

Copy link
Member

Choose a reason for hiding this comment

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

so IMO a whatsnew notes should not be added

OK, but on the off-chance that the Int64Index removal doesn't happen in time, pls be sure to add a note.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍

with pytest.raises(ValueError, match=msg):
self._index_cls(data)

def test_view_index(self, simple_index):
Expand Down Expand Up @@ -372,6 +367,10 @@ def simple_index(self, dtype):
def index(self, request, dtype):
return self._index_cls(request.param, dtype=dtype)

def test_constructor_from_list_no_dtype(self):
index = self._index_cls([1, 2, 3])
assert index.dtype == np.int64

def test_constructor(self, dtype):
index_cls = self._index_cls

Expand Down