implement Timedelta mod, divmod, rmod, rdivmod, fix and test scalar methods
State: closed

Implemented mod, divmod, rmod, rdivmod for Timedelta, along with tests.  In the process turned up a few bugs.  Conditional probability that there are more bugs is pretty good.

- fixed offsets.Tick // Timedelta, Timedelta // offsets.Tick
- fixed Timedelta [+-] timedelta64 returning timedelta64 instead of Timedelta
- fixed Timedelta [*/] int64 returning timedelta64 instead of Timedelta

- [ ] closes #xxxx
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry


Labels:
name: Testing
description: pandas testing functions or related to the test suite
name: Timedelta
description: Timedelta data type

Comments:
> Conditional probability that there are more bugs is pretty good

There are always more bugs 😄 

Also, you probably should add a `whatsnew` to address all of the things that you patched.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=h1) Report
> Merging [#19365](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/44c822de8dee0bf0e1ed2e8bc15424bb323b786f?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `n/a`.

[![Impacted file tree graph](https://codecov.io/gh/pandas-dev/pandas/pull/19365/graphs/tree.svg?height=150&src=pr&token=eZ4WkYLtcO&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #19365      +/-   ##
==========================================
- Coverage   91.58%   91.58%   -0.01%     
==========================================
  Files         150      150              
  Lines       48862    48862              
==========================================
- Hits        44750    44748       -2     
- Misses       4112     4114       +2
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.95% <ø> (-0.01%)` | :arrow_down: |
| #single | `41.75% <ø> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/19365/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `83.64% <0%> (-0.21%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=footer). Last update [44c822d...c22dc47](https://codecov.io/gh/pandas-dev/pandas/pull/19365?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

also see if we have any open issues related to this (maybe)
Thoughts on this?  There are other dark corners of Timedelta I'd like to address after this is done.
can you rebase, haven't looked in a while at this.
> is it easy to just return a TDI? this is very unituitive

That may become viable if/when TimedeltaArray becomes a thing.  For now 2D cases alone are a deal-breaker.
> That may become viable if/when TimedeltaArray becomes a thing. For now 2D cases alone are a deal-breaker.

2D should be banned. return a TDI.
> 2D should be banned. return a TDI.

Let's call this out-of-scope.  I'm not comfortable with this.  Consistency would require returning Index object wherever we currently return arrays.  Plus it isn't obvious that an immutable object should be returned.
then make all of these NotImplemented for now (timedelta and integer ndarray). This expands the scope too much here. 
> then make all of these NotImplemented for now (timedelta and integer ndarray). This expands the scope too much here.

This PR became necessary because I was trying to fix Index/Series ops wth timedelta-like.  Making these ops work is preliminary to make those work.
and ndarray integers arrays are bogus here. so make those raise.
For which ops are int-dtyped ndarrays bogus?
 \>1 ndim
So you want `Timedelta(minutes=1) * ndarray([1, 2], dtype=np.int64)` to raise?  That seems really counterintuitive.
greater than 1 d

that should return a TDI
@jorisvandenbossche can you weigh in on this?  I'm really uncomfortable with

a) returning TDI in timedelta64[ns] cases but ndarray in datetime/int/float cases
b) returning an immutable object (TDI) when the user didn't ask for one
c) having things that "just work" for np.timedelta64 start raising when wrapped with Timedelta (operations with ndim>1 ndarrays)
d) dropping a wedge between `td.__op__(frame)` vs `td.__op__(frame.values)`
@jbrockmendel you summary is not correct. an operation with a TDI and a 1D array should return a TDI. Not sure where your others points are coming from.
Can you open this discussion in a separate issue?  This fixes specific bugs and is a blocker for fixing other specific bugs, both of which I'd rather focus on.

> Not sure where your others points are coming from.

You told me to make ops with 2D ndarray raise.  Of course that would cause things to break that didn't in the past.

I'll give you half a point for d) as that was incorrect.  The requested change would _not_ introduce a wedge between `Timedelta.__op__(DataFrame)` vs `Timedelta.__op__(DataFrame.values)`.  It would cause _both_ to raise.
> @jbrockmendel you summary is not correct. an operation with a TDI and a 1D array should return a TDI.

@jreback not sure if it was a typo or not, but I think the discussion is about Timedelta * ndarray, not TimedeltaIndex * ndarray (I agree that last case should return TimedeltaIndex, but that is already the case right now)

We consistently return ndarray of datetime64 / timedelta64 on operations with Timedelta scalars and ndarrays. So I agree that *if* we want to change this, we should do it for all cases, so let's keep that for a separate issue, and keep this PR focused on the original issue?
OK, just implemented ~ the suggested changes.  I'd like to do another pass after this to flesh out the tests for Timedelta arithmetic ops.  If this doesn't pass muster as is, I think the next step is to strip out everything _except_ the mod/divmod and push the other arithmetic edits+tests to a separate PR.

@jorisvandenbossche Thanks for chiming in.  After looking at it I agree the handling without the `ndim==0` checks is prettier.  You get two internet points.
> so you haven't answered my question here. this needs to return a TDI or raise TypeError. return a m8 array is simply not useful.

I did answer by disagreeing and asking @jorisvandenbossche to weigh in.  Why would an m8 array not be useful?  The user clearly thinks its useful enough to pass as an input.  
Closing in favor of #19755, #19752, plus followups.