From 1.5.0 it appears that pd.Timedelta behavior has changed when used as an Enum data-type...
Before:
>>> import pandas as pd
>>> pd.__version__
'1.4.3'
>>> from enum import Enum
>>> class TDEnum(pd.Timedelta, Enum):
... T1 = 60000000000
...
... def plus_one_hour(self):
... return self + pd.Timedelta(hours=1)
...
>>> TDEnum["T1"] # as is usual when defining a data-type, returns enum...
<TDEnum.T1: Timedelta('0 days 00:01:00')>
>>> # ...such that methods defined on the enum are available...
>>> TDEnum["T1"].plus_one_hour()
Timedelta('0 days 01:01:00')
1.5.0:
>>> import pandas as pd
>>> pd.__version__
'1.5.0'
>>> # class defined as before...
>>> from enum import Enum
>>> class TDEnum(pd.Timedelta, Enum):
... T1 = 60000000000
...
... def plus_one_hour(self):
... return self + pd.Timedelta(hours=1)
...
>>> TDEnum["T1"] # now returns the pd.Timedelta enum value
Timedelta('0 days 00:01:00')
>>> # i.e. methods defined on the enum are no longer available
>>> TDEnum["T1"].plus_one_hour()
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
AttributeError: 'Timedelta' object has no attribute 'plus_one_hour'
I was reluctant to report this as a bug, although it is true that the Timedelta class appears to no longer behave as would be expected in this case (the usual behavior when defining a data-type (including datetime.timedelta) is as the 1.4.3 behavior described above).
If this is considered a bug, any chance it can be fixed?
If it's not a bug, might you have any ideas as to how I could replicate the previous behavior?
Thank you!
Comment From: MarcoGorelli
Thanks @maread99 for the report
Haven't looked into why yet, but from git bisect this is due to https://github.com/pandas-dev/pandas/pull/46688
Comment From: MarcoGorelli
@maread99 does this solve the issue in pandas?
Comment From: maread99
Nooooo! Reopened.
Sorry. I referenced this issue in a local PR. The PR resolved my issue locally by changing the enum data-type from pd.Timedelta to timedelta. Looks like GH picked up on the 'resolves' keyword there (before conversation there was edited) and closed the issue here!
Comment From: phofl
cc @jbrockmendel
Comment From: jbrockmendel
I think what happened is that #46688 changed Timedelta.__new__ to use _timedelta_from_value_and_reso which hard-codes Timedelta instead of using cls, so it does not respect subclasses. If that's it, then patching _timedelta_from_value_and_reso should solve this
Comment From: krasch
@jbrockmendel It was indeed _timedelta_from_value_and_reso that did not respect subclasses. Please have a look at my draft PR where I am patching it to restore the previous behaviour. Some open questions:
-
Is there still a need for a separate
timedelta_from_value_and_reso? It now has exactly the same signature as the classmethod_from_value_and_reso. The name might also be confusing now, if it is "timedelta_from_something", readers might expect the resulting object to be of class Timedelta and wonder why they need to pass a cls argument. -
Operations like the + operation in the initial example above will always return a Timedelta, not an object of the subclass. Seems like this would be expected behavior, otherwise the subclass would need to implement it's own add, I assume?
-
There seem to be very few tests, e.g. I could not find tests for the different operations mentioned in question 2 (so I do not actually know that they all return Timedelta, I just hope/assume). Should I add such tests even if they are somewhat out of the scope of this PR? I am happy to do so and they would make me feel more trusting towards the changes I made.
And a maybe more general question: is pandas generally expected to respect subclasses? I can imagine that the codebase contains many more such situations and that consistent behaviour might be hard to achieve.
(also, this was my first time working with cython, so please let me know if I went completely wrong somewhere)
Comment From: krasch
take