BUG: Behaviour change in 1.5.0 when using Timedelta as Enum data type #49579
Conversation
|
pls add tests where appropriate |
done |
| @@ -1829,6 +1829,7 @@ class Timedelta(_Timedelta): | |||
| return NaT | |||
|
|
|||
| return _timedelta_from_value_and_reso( | |||
| Timedelta, | |||
There was a problem hiding this comment.
should this be type(self)?
There was a problem hiding this comment.
same for other arithmetic methods above
There was a problem hiding this comment.
I had added a question re: this but somehow github ate it, weird..
Anyway, here is my concern, if I make the change you are suggesting
td1 = MyTimedeltaSubclass("10 minutes")
td2 = Timedelta ("10 minutes")
then result types will be inconsistent
td1 + td2 -> MyTimedeltaSubclass
td2 + td1 -> Timedelta
Maybe that is fine, but it also kinda weirded me out, which is why I wanted to ask you. It feels a little bit like it should be the responsibility of the subclass to implement the custom __add__ behaviour.
Wondering: does the rest of pandas respect subclasses in this way?
There was a problem hiding this comment.
does the rest of pandas respect subclasses in this way?
i dont think we're very consistent about it. IIRC the stdlib timedelta has something like:
def __add__(self, other):
if self_is_timedelta and other_is_timedelta_subclass:
return NotImplemented
[...]
which we could emulate. let's stick a pin in that for now and can keep Timedelta like you currently have
There was a problem hiding this comment.
Should I go ahead and create an issue for this?
|
Can you add a note in the 1.5.2 file |
krasch
changed the title
mroeschke
added
Regression
|
thanks @krasch |
|
Oops, something went wrong applying the patch ... Please have a look at my logs. |
…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
…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>
TimedeltaasEnumdata type #48908doc/source/whatsnew/vX.X.X.rstfile if fixing a bug or adding a new feature.