Skip to content
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

TST(string dtype): Resolve xfail in test_base.py #60713

Merged
merged 1 commit into from
Jan 14, 2025
+8 −6
Merged

TST(string dtype): Resolve xfail in test_base.py #60713

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions pandas/core/arrays/string_.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,6 +533,11 @@ def _str_map_nan_semantics(
else:
return self._str_map_str_or_object(dtype, na_value, arr, f, mask)

def view(self, dtype: Dtype | None = None) -> ArrayLike:
Copy link
Member

Choose a reason for hiding this comment

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

Overriding this here, the only thing it does is change the error from NotImplementedError to TypeError compared to the implementation in the base class?

Copy link
Member

Choose a reason for hiding this comment

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

Ah, for non-Arrow StringArray, this avoids getting the NDArrayBackedExtensionArray.view implemetation, which has different behaviour compared to the base class Extensionarray.view

if dtype is not None:
raise TypeError("Cannot change data-type for string array.")
Comment on lines +537 to +538
Copy link
Member Author

Choose a reason for hiding this comment

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

Are there any dtypes we do want to accept here?

Copy link
Member

Choose a reason for hiding this comment

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

I don't think so - the point of NumPy's view method seems incompatible with the string data types (both object-based and Arrow-based)

return super().view(dtype=dtype)
Copy link
Member

Choose a reason for hiding this comment

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

As an aside, I noticed NumPy's documentation on view says:

Passing None for dtype is different from omitting the parameter, since the former invokes dtype(None) which is an alias for dtype('float64').

So I'm assuming we overload the meaning of dtype=None in our extension class hierarchy somewhere, otherwise this would fail (?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Not sure what you mean by "otherwise this would fail". I think we have behavior that differs from NumPy here:

pandas/pandas/core/arrays/base.py

Lines 1874 to 1876 in fa5c255

if dtype is not None:
raise NotImplementedError(dtype)
return self[:]

Copy link
Member

Choose a reason for hiding this comment

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

OK good to know we override. It wouldn't make sense otherwise to reinterpret these bytes using NumPy's default behavior (float)



# error: Definition of "_concat_same_type" in base class "NDArrayBacked" is
# incompatible with definition in base class "ExtensionArray"
Expand Down
9 changes: 3 additions & 6 deletions pandas/tests/indexes/test_base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -351,14 +351,11 @@ def test_view_with_args_object_array_raises(self, index):
msg = "When changing to a larger dtype"
with pytest.raises(ValueError, match=msg):
index.view("i8")
elif index.dtype == "str" and not index.dtype.storage == "python":
# TODO(infer_string): Make the errors consistent
with pytest.raises(NotImplementedError, match="i8"):
index.view("i8")
else:
msg = (
"Cannot change data-type for array of references.|"
"Cannot change data-type for object array.|"
r"Cannot change data-type for array of references\.|"
r"Cannot change data-type for object array\.|"
r"Cannot change data-type for array of strings\.|"
)
with pytest.raises(TypeError, match=msg):
index.view("i8")
Expand Down
Loading