[ArrowStringArray] fix test_astype_int, test_astype_float #41018
Conversation
pandas/core/arrays/string_arrow.py
Outdated
| return self | ||
|
|
||
| elif hasattr(dtype, "__from_arrow__"): | ||
| return dtype.__from_arrow__(self._data) |
There was a problem hiding this comment.
I don't think we should use this method like this: __from_arrow__ is meant to convert data from arrow for that specific dtype, and doesn't necessarily need to include any casting logic (I think)
There was a problem hiding this comment.
Alternative logic could be something like:
elif isinstance(dtype, NumericDtype):
data = self._data.cast(pa.from_numpy_dtype(dtype.numpy_dtype))
return dtype.__from_arrow__(data)
that would specifically work for the numeric masked arrays, and not rely on __from_arrow__ to do the casting (but rely on pyarrow for that).
This would already support casting to nullable integer / float to get the tests passing.
An example why we can rely in general on __from_arrow__ is eg if we would do here string_array.astype(pd.PeriodDtype("D")), which would fail in ``from_arrow
There was a problem hiding this comment.
An example why we can rely in general on
__from_arrow__is eg if we would do herestring_array.astype(pd.PeriodDtype("D")), which would fail in ``from_arrow
indeed, we have a similar with StringArray as well #40566 where
elif isinstance(dtype, ExtensionDtype):
cls = dtype.construct_array_type()
return cls._from_sequence(self, dtype=dtype, copy=copy)
in many cases fails as _from_sequence, like __from_arrow__, does not support casting in many cases.
|
the changes to to_numpy are implicitly tested (and different behavior to StringArray). will need to add tests to explicitly tests this with the changes requested here. will open separate PRs for to_numpy/astype and close this to clear the queue. |
| @@ -737,6 +745,24 @@ def value_counts(self, dropna: bool = True) -> Series: | |||
|
|
|||
| return Series(counts, index=index).astype("Int64") | |||
|
|
|||
| def astype(self, dtype, copy=True): | |||
There was a problem hiding this comment.
can you type here (followups are ok)
| expected = np.array([1, 2, 3], dtype="int64") | ||
| tm.assert_numpy_array_equal(result, expected) | ||
|
|
||
| arr = pd.array(["1", pd.NA, "3"], dtype=dtype) |
There was a problem hiding this comment.
prob best in a dedicate _errors test
There was a problem hiding this comment.
sure. done in simonjayhawkins@e1577d4
will open a PR with other follow-ups
|
merging, @simonjayhawkins if you can note the followups |
sure. The typing is included in #35169 (comment) and will be done after the work by @Dr-Irv sorting out the base EA types. |
No description provided.