API: Copy inputs in Index subclass constructors by default (GH#63388) #63398
Conversation
|
All checks are passing now. I've updated the PR to reflect the latest changes. |
pandas/core/indexes/datetimes.py
Outdated
| if isinstance(data, (ExtensionArray, np.ndarray)): | ||
| # GH 63388 | ||
| if copy is not False: | ||
| if dtype is None or astype_is_view( | ||
| data.dtype, | ||
| pandas_dtype(dtype), | ||
| ): | ||
| data = data.copy() | ||
| copy = False |
There was a problem hiding this comment.
Can you create a @classmethod (place it in indexes/base.py near _raise_scalar_data_error), that encapsulates this logic and can be called here instead?
Extracts the copy/view validation logic into a shared classmethod on the base Index class, as requested during review.
Updates DatetimeIndex, TimedeltaIndex, PeriodIndex, and IntervalIndex to use the shared validation logic, reducing code duplication.
|
Done. I refactored the logic into a |
| :meth:`~DataFrame.ffill`, :meth:`~DataFrame.bfill`, :meth:`~DataFrame.interpolate`, | ||
| :meth:`~DataFrame.where`, :meth:`~DataFrame.mask`, :meth:`~DataFrame.clip`) now return | ||
| the modified DataFrame or Series (``self``) instead of ``None`` when ``inplace=True`` (:issue:`63207`) | ||
| - :class:`DatetimeIndex`, :class:`TimedeltaIndex`, :class:`PeriodIndex` and :class:`IntervalIndex` constructors now copy the input ``data`` by default when ``copy=None``, consistent with :class:`Index` behavior (:issue:`63388`) |
There was a problem hiding this comment.
Could you remove this note (I'll probably be adding something related to this in the copy on write guide)
| ) | ||
|
|
||
| @classmethod | ||
| def _maybe_copy_input(cls, data, copy, dtype): |
There was a problem hiding this comment.
| def _maybe_copy_input(cls, data, copy, dtype): | |
| def _maybe_copy_array_input(cls, data, copy: bool | None, dtype) -> tuple[Any, bool]: |
| from pandas.core.dtypes.common import ( | ||
| is_integer, | ||
| ) |
There was a problem hiding this comment.
Can you undo these import changes if the contents haven't changed in your PR?
| if dtype is None or astype_is_view(data.dtype, pandas_dtype(dtype)): | ||
| data = data.copy() | ||
| copy = False | ||
| return data, copy |
There was a problem hiding this comment.
| return data, copy | |
| return data, bool(copy) |
Then you can remove the bool(copy) in other files
2 participants
AGENTS.md.Description:
This PR aligns the behavior of
DatetimeIndex,TimedeltaIndex,PeriodIndex, andIntervalIndexwith the baseIndexclass (andSeries) regarding thecopyparameter.Before, these constructors would ignore
copy=Noneand default to not copying the input data.Now,
copy=Nonedefaults toTruefornp.ndarrayandExtensionArrayinputs, ensuring that the resulting Index does not share memory with the mutable input array by default.Changes:
__new__signatures indatetimes.py,timedeltas.py,period.py, andinterval.pyto acceptcopy: bool | None.copyisNone(orTrue) and the input is an array/EA.Indexdocstring for thecopyparameter.dtypearguments (e.g."datetime64[ns]") are converted toDtypeObjbefore the view checks, preventingAttributeError.test_constructor_int64_nocopyandtest_array_tz) to explicitly passcopy=False. Since the default behavior changed tocopy=True, these tests required an update to continue verifying that the view based construction is still possible when requested.