TYP: tighter typing in Offset._apply

[jbrockmendel]

Rename _apply -> _add_datetime and type it as requiring/returning a datetime object.

This currently fails on TestBusinessDay::test_apply_corner where it raises RecursionError instead of TypeError. I could use some help figuring out why (cc @WillAyd?) Roughly:

off = pd.offsets.BDay()
other = pd.BMonthEnd()
with pytest.raises(TypeError, match=msg):
>    off + other

pandas/tests/tseries/offsets/test_business_day.py:237: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
offsets.pyx:1715: in pandas._libs.tslibs.offsets.BusinessDay.__add__
    ???
offsets.pyx:495: in pandas._libs.tslibs.offsets.BaseOffset.__radd__
    ???
offsets.pyx:495: in pandas._libs.tslibs.offsets.BaseOffset.__radd__
    ???
E   RecursionError: maximum recursion depth exceeded while calling a Python object
!!! Recursion detected (same locals & position)

IIUC BusinessDay.__add__ is (correctly) returning NotImplemented, so BMonthEnd.__radd__ is called, which also returns NotImplemented. But instead of python then raising TypeError, it looks like it just keeps calling __radd__. Am I wrong to expect python to raise TypeError when __radd__ returns NotImplemented?

[WillAyd]

Hmm just a guess but I wonder if radd just dispatching to add causes the recursion. Does mul/rmul have the same issue?

[jbrockmendel]

Does mul/rmul have the same issue?

Good idea, yes it does! On main:

off = pd.offsets.Day()
off * off
[...]
  File "offsets.pyx", line 1018, in pandas._libs.tslibs.offsets.Tick.__mul__
  File "offsets.pyx", line 519, in pandas._libs.tslibs.offsets.BaseOffset.__rmul__
RecursionError: maximum recursion depth exceeded while calling a Python object

In 2.0.2 this multiplication correctly raised TypeError. Best guess for what changed is we got rid of some cython2-compat code for how it dispatched __r(add|mul|etc)__. @da-woods do we need to reinstate some special handling?

[da-woods]

I can reproduce it with the much simpler non-Pandas example:

cdef class C:
    def __add__(self, other):
        return NotImplemented

    def __radd__(self, other):
        return self.__add__(other)

I think (but haven't rigorously verified) that the problem is that we don't define __add__ in the class dictionary. This means that when you do self.__add__, it doesn't call the __add__ function as written, but instead calls the dispatch function that against tries __radd__.

I'll create a Cython bug report - cython/cython#5863. If it's what I've suggest above then it probably isn't too hard to fix.

[da-woods]

If you have code that you can reinstate and which worked then I'd do that until a Cython fix appears

[github-actions]

This pull request is stale because it has been open for thirty days with no activity. Please update and respond to this comment if you're still interested in working on this.

[mroeschke]

Looks like this has gone stale and needs a rebase. Closing to clear the queue but feel free to reopen when you want to circle back