BUG: Fix all-NaT when ArrowEA.astype to categorical #62055
Conversation
arthurlw
changed the title
arthurlw
changed the title
pandas/core/arrays/base.py
Outdated
| dtype = pandas_dtype(dtype) | ||
| if dtype == self.dtype: | ||
| if not copy: | ||
| return self | ||
| else: | ||
| return self.copy() | ||
|
|
||
| if isinstance(dtype, CategoricalDtype): |
There was a problem hiding this comment.
Pretty weird to do this in the base class. Can you track down where the actual problem is? Could be hiding other bugs eg in categorical.fromsequence
There was a problem hiding this comment.
Ah okay i'll investigate further
pandas/core/arrays/categorical.py
Outdated
| if is_datetime64_any_dtype(cat_dtype) or is_timedelta64_dtype( | ||
| cat_dtype | ||
| ): | ||
| values = values.to_numpy() |
There was a problem hiding this comment.
this can be expensive, particularly in dt64tz. have you figured out why the problem is happening? i.e. if you step through the code, where does the first wrong result show up
|
Using In the first example, the code does not think they are comparable in |
|
So we should start by fixing should_compare? |
Sorry I missed this earlier. The fix I pushed addresses that and resolves the second issue too. |
| @@ -3674,6 +3674,14 @@ def get_indexer( | |||
| orig_target = target | |||
| target = self._maybe_cast_listlike_indexer(target) | |||
|
|
|||
| from pandas.api.types import is_timedelta64_dtype | |||
There was a problem hiding this comment.
this can go at the top of the file
| @@ -3674,6 +3674,14 @@ def get_indexer( | |||
| orig_target = target | |||
| target = self._maybe_cast_listlike_indexer(target) | |||
|
|
|||
| from pandas.api.types import is_timedelta64_dtype | |||
|
|
|||
| if target.dtype == "string[pyarrow]" and is_timedelta64_dtype(self.dtype): | |||
There was a problem hiding this comment.
i dont think we generally do this implicit casting anymore
| @@ -384,6 +384,18 @@ def _is_comparable_dtype(self, dtype: DtypeObj) -> bool: | |||
| if self.tz is not None: | |||
| # If we have tz, we can compare to tzaware | |||
| return isinstance(dtype, DatetimeTZDtype) | |||
|
|
|||
| from pandas import ArrowDtype | |||
There was a problem hiding this comment.
can go at the top of the file
|
|
||
| return ( | ||
| pa.types.is_date32(dtype.pyarrow_dtype) | ||
| or pa.types.is_date64(dtype.pyarrow_dtype) |
There was a problem hiding this comment.
i think timestamp is comparable but date is not
| @@ -160,3 +163,20 @@ def test_astype_category_readonly_mask_values(self): | |||
| result = arr.astype("category") | |||
| expected = array([0, 1, 2], dtype="Int64").astype("category") | |||
| tm.assert_extension_array_equal(result, expected) | |||
|
|
|||
| def test_arrow_array_astype_to_categorical_dtype_temporal(self): | |||
There was a problem hiding this comment.
can you also test the intermediate steps that used to fail
Added type annotations to new arguments/methods/functions.doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.