Skip to content

DEPS: Update reprs with numpy NEP51 scalar repr #54268

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

Merged
merged 3 commits into from
Jul 28, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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: 2 additions & 1 deletion pandas/_libs/interval.pyx
Original file line number Diff line number Diff line change
Expand Up @@ -492,8 +492,9 @@ cdef class Interval(IntervalMixin):
def __repr__(self) -> str:

left, right = self._repr_base()
disp = str if isinstance(left, np.generic) else repr
name = type(self).__name__
repr_str = f"{name}({repr(left)}, {repr(right)}, closed={repr(self.closed)})"
repr_str = f"{name}({disp(left)}, {disp(right)}, closed={repr(self.closed)})"
Copy link
Member

Choose a reason for hiding this comment

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

maybe the thing to do is unpack numpy scalars at an earlier stage?

Copy link
Member Author

Choose a reason for hiding this comment

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

But we should still store numpy scalars for left/right on Interval right?

Copy link
Member

Choose a reason for hiding this comment

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

i was thinking store native scalars

Copy link
Member Author

Choose a reason for hiding this comment

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

Hmm might make sense, not sure if there's any larger ramifications of that chance. Might be better to experiment in a follow up?

Copy link
Member

Choose a reason for hiding this comment

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

sure

return repr_str

def __str__(self) -> str:
Expand Down
4 changes: 4 additions & 0 deletions pandas/core/arrays/masked.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
from typing import (
TYPE_CHECKING,
Any,
Callable,
Literal,
overload,
)
Expand Down Expand Up @@ -160,6 +161,9 @@ def _empty(cls, shape: Shape, dtype: ExtensionDtype):
)
return result

def _formatter(self, boxed: bool = False) -> Callable[[Any], str | None]:
return super()._formatter(boxed=True)
Copy link
Member

Choose a reason for hiding this comment

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

we're ignoring the boxed keyword?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah since I think we'll need to always return str given the NEP 51 change. Actually probably just simpler to return str here


@property
def dtype(self) -> BaseMaskedDtype:
raise AbstractMethodError(self)
Expand Down
5 changes: 3 additions & 2 deletions pandas/core/indexes/base.py
Original file line number Diff line number Diff line change
Expand Up @@ -5250,7 +5250,8 @@ def _raise_scalar_data_error(cls, data):
# in order to keep mypy happy
raise TypeError(
f"{cls.__name__}(...) must be called with a collection of some "
f"kind, {repr(data)} was passed"
f"kind, {repr(data) if not isinstance(data, np.generic) else str(data)} "
Copy link
Member

Choose a reason for hiding this comment

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

does str work unconditionally?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah according to the linked PR seems so

Copy link
Member

Choose a reason for hiding this comment

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

the can we simplify this?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh sorry, str works unconditionally for numpy scalars but repr vs str would make this display differently for other scalars e.g. python strings

"was passed"
)

def _validate_fill_value(self, value):
Expand Down Expand Up @@ -6974,7 +6975,7 @@ def drop(
mask = indexer == -1
if mask.any():
if errors != "ignore":
raise KeyError(f"{list(labels[mask])} not found in axis")
raise KeyError(f"{labels[mask].tolist()} not found in axis")
indexer = indexer[~mask]
return self.delete(indexer)

Expand Down
1 change: 1 addition & 0 deletions pandas/tests/arrays/floating/test_construction.py
Original file line number Diff line number Diff line change
Expand Up @@ -125,6 +125,7 @@ def test_to_array_error(values):
"Cannot pass scalar",
r"float\(\) argument must be a string or a (real )?number, not 'dict'",
"could not convert string to float: 'foo'",
r"could not convert string to float: np\.str_\('foo'\)",
]
)
with pytest.raises((TypeError, ValueError), match=msg):
Expand Down
2 changes: 1 addition & 1 deletion pandas/tests/arrays/sparse/test_dtype.py
Original file line number Diff line number Diff line change
Expand Up @@ -197,7 +197,7 @@ def test_update_dtype(original, dtype, expected):
(
SparseDtype(str, "abc"),
int,
re.escape("invalid literal for int() with base 10: 'abc'"),
r"invalid literal for int\(\) with base 10: ('abc'|np\.str_\('abc'\))",
),
],
)
Expand Down