Skip to content

Commit

Permalink
BUG: Behaviour change in 1.5.0 when using Timedelta as Enum data type (
Browse files Browse the repository at this point in the history
…pandas-dev#49579)

* Add cls argument to _timedelta_from_value_and_reso

* Add test

* Add fix to changelog

Co-authored-by: krasch <dev@krasch.io>
(cherry picked from commit 606499d)

# Conflicts:
#	pandas/_libs/tslibs/timedeltas.pyx
#	pandas/tests/scalar/timedelta/test_constructors.py
  • Loading branch information
krasch authored and phofl committed Nov 19, 2022
1 parent 9196f8d commit 7da0091
Show file tree
Hide file tree
Showing 3 changed files with 22 additions and 12 deletions.
2 changes: 1 addition & 1 deletion doc/source/whatsnew/v1.5.2.rst
Expand Up @@ -20,7 +20,7 @@ Fixed regressions
from being passed using the ``colormap`` argument if Matplotlib 3.6+ is used (:issue:`49374`)
- Fixed regression in :func:`date_range` returning an invalid set of periods for ``CustomBusinessDay`` frequency and ``start`` date with timezone (:issue:`49441`)
- Fixed performance regression in groupby operations (:issue:`49676`)
-
- Fixed regression in :class:`Timedelta` constructor returning object of wrong type when subclassing ``Timedelta`` (:issue:`49579`)

.. ---------------------------------------------------------------------------
.. _whatsnew_152.bug_fixes:
Expand Down
23 changes: 12 additions & 11 deletions pandas/_libs/tslibs/timedeltas.pyx
Expand Up @@ -188,7 +188,7 @@ def ints_to_pytimedelta(ndarray m8values, box=False):
res_val = <object>NaT
else:
if box:
res_val = _timedelta_from_value_and_reso(value, reso=reso)
res_val = _timedelta_from_value_and_reso(Timedelta, value, reso=reso)
elif reso == NPY_DATETIMEUNIT.NPY_FR_ns:
res_val = timedelta(microseconds=int(value) / 1000)
elif reso == NPY_DATETIMEUNIT.NPY_FR_us:
Expand Down Expand Up @@ -737,7 +737,7 @@ cdef bint _validate_ops_compat(other):
def _op_unary_method(func, name):
def f(self):
new_value = func(self.value)
return _timedelta_from_value_and_reso(new_value, self._reso)
return _timedelta_from_value_and_reso(Timedelta, new_value, self._reso)
f.__name__ = name
return f

Expand Down Expand Up @@ -807,7 +807,7 @@ def _binary_op_method_timedeltalike(op, name):
# TODO: more generally could do an overflowcheck in op?
return NaT

return _timedelta_from_value_and_reso(res, reso=self._reso)
return _timedelta_from_value_and_reso(Timedelta, res, reso=self._reso)

f.__name__ = name
return f
Expand Down Expand Up @@ -938,10 +938,10 @@ cdef _to_py_int_float(v):


def _timedelta_unpickle(value, reso):
return _timedelta_from_value_and_reso(value, reso)
return _timedelta_from_value_and_reso(Timedelta, value, reso)


cdef _timedelta_from_value_and_reso(int64_t value, NPY_DATETIMEUNIT reso):
cdef _timedelta_from_value_and_reso(cls, int64_t value, NPY_DATETIMEUNIT reso):
# Could make this a classmethod if/when cython supports cdef classmethods
cdef:
_Timedelta td_base
Expand All @@ -951,13 +951,13 @@ cdef _timedelta_from_value_and_reso(int64_t value, NPY_DATETIMEUNIT reso):
# We pass 0 instead, and override seconds, microseconds, days.
# In principle we could pass 0 for ns and us too.
if reso == NPY_FR_ns:
td_base = _Timedelta.__new__(Timedelta, microseconds=int(value) // 1000)
td_base = _Timedelta.__new__(cls, microseconds=int(value) // 1000)
elif reso == NPY_DATETIMEUNIT.NPY_FR_us:
td_base = _Timedelta.__new__(Timedelta, microseconds=int(value))
td_base = _Timedelta.__new__(cls, microseconds=int(value))
elif reso == NPY_DATETIMEUNIT.NPY_FR_ms:
td_base = _Timedelta.__new__(Timedelta, milliseconds=0)
td_base = _Timedelta.__new__(cls, milliseconds=0)
elif reso == NPY_DATETIMEUNIT.NPY_FR_s:
td_base = _Timedelta.__new__(Timedelta, seconds=0)
td_base = _Timedelta.__new__(cls, seconds=0)
# Other resolutions are disabled but could potentially be implemented here:
# elif reso == NPY_DATETIMEUNIT.NPY_FR_m:
# td_base = _Timedelta.__new__(Timedelta, minutes=int(value))
Expand Down Expand Up @@ -1532,7 +1532,7 @@ cdef class _Timedelta(timedelta):
@classmethod
def _from_value_and_reso(cls, int64_t value, NPY_DATETIMEUNIT reso):
# exposing as classmethod for testing
return _timedelta_from_value_and_reso(value, reso)
return _timedelta_from_value_and_reso(cls, value, reso)

def _as_unit(self, str unit, bint round_ok=True):
dtype = np.dtype(f"m8[{unit}]")
Expand Down Expand Up @@ -1708,7 +1708,7 @@ class Timedelta(_Timedelta):
if value == NPY_NAT:
return NaT

return _timedelta_from_value_and_reso(value, NPY_FR_ns)
return _timedelta_from_value_and_reso(cls, value, NPY_FR_ns)

def __setstate__(self, state):
if len(state) == 1:
Expand Down Expand Up @@ -1800,6 +1800,7 @@ class Timedelta(_Timedelta):
return NaT

return _timedelta_from_value_and_reso(
Timedelta,
<int64_t>(other * self.value),
reso=self._reso,
)
Expand Down
9 changes: 9 additions & 0 deletions pandas/tests/scalar/timedelta/test_constructors.py
Expand Up @@ -402,3 +402,12 @@ def test_string_without_numbers(value):
)
with pytest.raises(ValueError, match=msg):
Timedelta(value)


def test_subclass_respected():
# GH#49579
class MyCustomTimedelta(Timedelta):
pass

td = MyCustomTimedelta("1 minute")
assert isinstance(td, MyCustomTimedelta)

0 comments on commit 7da0091

Please sign in to comment.