Skip to content

BUG: Fix all-NaT when ArrowEA.astype to categorical #62055

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

Open
wants to merge 10 commits into
base: main
Choose a base branch
from
+41 −1
Open

BUG: Fix all-NaT when ArrowEA.astype to categorical #62055

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
58284dd
Added condition for CategoricalDtype
arthurlw Aug 6, 2025
1896199
Added tests
arthurlw Aug 6, 2025
36b84cf
whatsnew
arthurlw Aug 6, 2025
2dc0698
Added GH ref
arthurlw Aug 6, 2025
5987952
Updated conditions
arthurlw Aug 6, 2025
a5fab10
Updated fix logic
arthurlw Aug 6, 2025
e6b7c64
importorskip
arthurlw Aug 6, 2025
95e331c
Update fix logic
arthurlw Aug 12, 2025
3453487
precommit and removed unnecessary comments
arthurlw Aug 12, 2025
d747141
Update condition
arthurlw Aug 12, 2025
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
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v3.0.0.rst
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -693,7 +693,7 @@ Categorical
- Bug in :meth:`Categorical.astype` where ``copy=False`` would still trigger a copy of the codes (:issue:`62000`)
- Bug in :meth:`DataFrame.pivot` and :meth:`DataFrame.set_index` raising an ``ArrowNotImplementedError`` for columns with pyarrow dictionary dtype (:issue:`53051`)
- Bug in :meth:`Series.convert_dtypes` with ``dtype_backend="pyarrow"`` where empty :class:`CategoricalDtype` :class:`Series` raised an error or got converted to ``null[pyarrow]`` (:issue:`59934`)
-
- Bug in :meth:`array.astype` where casting a pyarrow-backed array to a temporal :class:`CategoricalDtype` (e.g. with datetime or timedelta categories) raised or incorrectly converted values to all ``NaT`` (:issue:`62051`)

Datetimelike
^^^^^^^^^^^^
Expand Down
8 changes: 8 additions & 0 deletions pandas/core/indexes/base.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

this can go at the top of the file


if target.dtype == "string[pyarrow]" and is_timedelta64_dtype(self.dtype):
Copy link
Member

Choose a reason for hiding this comment

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

i dont think we generally do this implicit casting anymore

Copy link
Member Author

Choose a reason for hiding this comment

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

that's fair, but the array is initialized as string[pyarrow] instead of a pyarrow time scalar. Not sure how to compare string with timestamp64 without some coercion step

Copy link
Member

Choose a reason for hiding this comment

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

Looking at this more closely, i was wrong about not casting strings.

tdi = pd.date_range("2016-01-01", periods=3) - pd.Timestamp("2016-01-01")
target = pd.array(["0 days"], dtype="string[pyarrow]")

>>> tdi.get_indexer(["0 days"])
array([0])
>>> tdi._maybe_cast_listlike_indexer(["0 days"])
TimedeltaIndex(['0 days'], dtype='timedelta64[ns]', freq=None)
>>> tdi._maybe_cast_listlike_indexer(target)
TimedeltaIndex(['0 days'], dtype='timedelta64[ns]', freq=None)

If a patch is needed, can it go in _maybe_cast_listlike_indexer?

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 i was thinking about using a maybe helper function too

from pandas.core.arrays.timedeltas import sequence_to_td64ns

data, freq = sequence_to_td64ns(target, copy=False, unit=None)
target = type(target)(data)

self._check_indexing_method(method, limit, tolerance)

if not self._index_as_unique:
Expand Down
12 changes: 12 additions & 0 deletions pandas/core/indexes/datetimes.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Copy link
Member

Choose a reason for hiding this comment

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

can go at the top of the file


if isinstance(dtype, ArrowDtype):
import pyarrow as pa

return (
pa.types.is_date32(dtype.pyarrow_dtype)
or pa.types.is_date64(dtype.pyarrow_dtype)
Copy link
Member

Choose a reason for hiding this comment

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

i think timestamp is comparable but date is not

Copy link
Member Author

Choose a reason for hiding this comment

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

The original issue was with pyarrow date dtypes, which compare fine when using astype, so I think they should be treated as comparable here

Copy link
Member

Choose a reason for hiding this comment

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

dti = pd.date_range("2016-01-01", periods=3)
item = dti[0].date()
>>> (item == dti)[0]
np.False_

We don't have a non-pyarrow date dtype, but if we did, it would not be considered comparable to datetime64

Copy link
Member Author

Choose a reason for hiding this comment

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

I think in that case the question is whether we want astype with categoricals to succeed here, or whether astype between pyarrow date and datetime64 should be disallowed for consistency

Copy link
Member

Choose a reason for hiding this comment

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

do we have analogous special-casing for the non-pyarrow dt64 that im missing?

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 that I know of

Copy link
Member

Choose a reason for hiding this comment

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

Then I expect it shouldn’t be necessary here. I’ll take a closer look on monday

Copy link
Member

Choose a reason for hiding this comment

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

i think the relevant special-casing is in Index._maybe_downcast_for_indexing. Take a look for the inferred_type checks

or pa.types.is_timestamp(dtype.pyarrow_dtype)
)

# if we dont have tz, we can only compare to tznaive
return lib.is_np_dtype(dtype, "M")

Expand Down
20 changes: 20 additions & 0 deletions pandas/tests/arrays/categorical/test_astype.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,17 @@
CategoricalDtype,
CategoricalIndex,
DatetimeIndex,
Index,
Interval,
NaT,
Period,
Timestamp,
array,
isna,
to_datetime,
)
import pandas._testing as tm
from pandas.core.arrays.arrow.array import ArrowExtensionArray


class TestAstype:
Expand Down Expand Up @@ -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):
Copy link
Member

Choose a reason for hiding this comment

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

can you also test the intermediate steps that used to fail

# GH#62051
pytest.importorskip("pyarrow")
arr = array(
["2017-01-01", "2018-01-01", "2019-01-01"], dtype="date32[day][pyarrow]"
)
cats = Index(["2017-01-01", "2018-01-01", "2019-01-01"], dtype="M8[s]")
dtype = CategoricalDtype(categories=cats, ordered=False)

assert not all(isna(arr.astype(dtype)))

arr = ArrowExtensionArray._from_sequence(["1h", "2h", "3h"])
cats = Index(["1h", "2h", "3h"], dtype="m8[ns]")
dtype = CategoricalDtype(cats, ordered=False)

assert not all(isna(arr.astype(dtype)))
Loading