Port Timedelta implementation to tslibs.timedeltas

[jbrockmendel]

Cut/paste Timedelta and _Timedelta methods into tslibs.timesdeltas._Timedelta. The moved methods are copied verbatim.

Timedelta._binary_op_method_timedeltalike and Timedelta._op_unary_method do not use self, are moved out of the class definitions (but stay in tslib). (In a follow-up _op_unary_method can be moved to tslibs.timedeltas, _binary_op_method_timedeltalike cannot.)

Most of the rest of _Timedelta&Timedelta can be moved into tslibs.timedeltas._Timedelta after #17793.

On the side, cleanup in core.indexes of redundant isinstance checks.

• closes #xxxx
• tests added / passed
• passes git diff upstream/master -u -- "*.py" | flake8 --diff
• whatsnew entry

[jreback]

is this mostly cut/paste?

[jbrockmendel]

is this mostly cut/paste?

Yes. Some methods in Timedelta are moved into the cdefd _Timedelta, but their implementations are unchanged. Timedelta._op_unary_method and Timedelta._binary_op_method_timedeltalike are made module-level functions instead of methods, but their implementations are unchanged.

The only actual changes are in core.indexes, where isinstance(foo, (datetime, Timestamp)) is changed to isinstance(foo, datetime), ditto for (timedelta, Timedelta) --> timedelta.

There are some non-trivial upcoming changes, but I'm trying to make your life easier by not mixing those in with cut/paste edits.

[codecov]

Codecov Report

Merging #17937 into master will decrease coverage by 0.01%.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           master   #17937      +/-   ##
==========================================
- Coverage   91.25%   91.23%   -0.02%     
==========================================
  Files         163      163              
  Lines       50120    50120              
==========================================
- Hits        45737    45728       -9     
- Misses       4383     4392       +9

Flag	Coverage Δ
#multiple	89.04% <ø> (ø)	⬆️
#single	40.32% <ø> (-0.06%)	⬇️

Impacted Files	Coverage Δ
pandas/io/gbq.py	25% <0%> (-58.34%)	⬇️
pandas/core/frame.py	97.75% <0%> (-0.1%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 15fa4bd...b527e9a. Read the comment docs.

[jreback]

i think need to fix the NaT things first rather than halfway so this
you can pull out the isintance changes into a separate PR

[jbrockmendel]

i think need to fix the NaT things first rather than halfway so this
you can pull out the isintance changes into a separate PR

Are you saying you want the edits in core.indexes in a separate PR? This is basically unrelated to #17793.

[jreback]

yes on both

[jbrockmendel]

OK, just reverted the changes in core.indexes, will put them in a separate PR.

[jbrockmendel]

Updated with one more method (_validate_ops_compat) moved out of the class and instead cdef'd as a function.

[jreback]

you might be able to move TImedelta completely to the new module as you now can import NaT directly w/o tslib and if you change the Timestamp checks to isinstance(obj, datetime).

[jreback]

Most of the rest of _Timedelta&Timedelta can be moved into tslibs.timedeltas._Timedelta after #17793.

actually you already stated this, hahah

[jbrockmendel]

Yep. I'd prefer to close this out and do the rest in a separate PR, but either way is OK.

[jreback]

I think let's move this all at once

[jbrockmendel]

OK. The all-at-once needs to wait for #18014.

The reason I prefer to do this in two steps is because the follow up can be pure cut/paste, whereas this contains nonzero other changes that have already been reviewed.

[jbrockmendel]

(also this re-fixes a bunch of flake8 errors that got un-fixed in one of my rebase screwups)

[jreback]

give this a rebase.

[jbrockmendel]

Suggestions? This is on-deck to be a blocker

[jreback]

I'd like to move the entire Timedelta impl out of tslib.pyx

[jbrockmendel]

I'd like to move the entire Timedelta impl out of tslib.pyx

So would I, but we're either going to have to settle for almost all of it or for having an ugly circular-esque import with Timestamp. If this gets merged I can get the rest of _Timedelta moved over later this evening.

[jbrockmendel]

The methods moved in this PR are cut/paste, whereas the other methods that need to be moved will take some tinkering, for a less trivial review process. There is value in separating these steps.

[jreback]

ok, then pls add to your list moving the rest of Timedelta. It can be entirely defined in timedeltas.

I think its ok to import it inside a function if needed, you can probably do isinstance checks on timedelta/datetime as needed as well (rather than Timedelta). you can also maybe just duck-type check for Timestamp e.g.

def is_timestamp(obj):
   return isinstance(obj, datetime) and hasattr(obj, 'value')

[jbrockmendel]

I think its ok to import it inside a function if needed

Great. I had assumed that wouldn't be acceptable performance-wise, but never bothered to measure it. That makes the remaining porting a lot simpler; pushing shortly.