WIP: NaTD
State: closed

There are a bunch of places where we do something like:

```
    def method(self, other):
        [...]
        if isinstance(other, (np.timedelta64, timedelta, Tick)):
            other = Timedelta(other)
       [...]
```

But in the case where we start with `np.timedelta64('NaT')`, we end up with `NaT` which is datetime-like instead of timedelta-like.  In some of the places where this occurs, we check for this case and special-case it.  In others we miss it completely.

I am _not_ proposing to change the behavior of `Timedelta` or make `NaTD` public.  The idea is that since we need the arithmetic/comparison methods anyway, we might as well put them into one place and handle them systematically.

Tests are a mess at the moment.

Labels:
name: Missing-data
description: np.nan, pd.NaT, pd.NA, dropna, isnull, interpolate
name: Timedelta
description: Timedelta data type

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=h1) Report
> Merging [#24645](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/ebd598a1dc30d0a156b7710808596bcc39e83b3e?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #24645      +/-   ##
==========================================
+ Coverage   92.37%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52379    52381       +2     
==========================================
+ Hits        48387    48392       +5     
+ Misses       3992     3989       -3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <100%> (ø)` | :arrow_up: |
| #single | `43.01% <10%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `88.73% <100%> (+0.54%)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.28% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.36% <100%> (+0.06%)` | :arrow_up: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `88% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24645?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/24645?src=pr&el=footer). Last update [ebd598a...d0efc5f](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=h1) Report
> Merging [#24645](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/393d0aa1dc1b2b0cd478cbded6fb13e530589c4e?src=pr&el=desc) will **increase** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #24645      +/-   ##
==========================================
+ Coverage   92.37%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52384    52386       +2     
==========================================
+ Hits        48390    48396       +6     
+ Misses       3994     3990       -4
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <100%> (ø)` | :arrow_up: |
| #single | `43.02% <10%> (ø)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `88.64% <100%> (+0.55%)` | :arrow_up: |
| [pandas/core/ops.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvb3BzLnB5) | `94.28% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/24645/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.36% <100%> (+0.06%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24645?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/24645?src=pr&el=footer). Last update [393d0aa...1b533ff](https://codecov.io/gh/pandas-dev/pandas/pull/24645?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).

-1 on this. I would rather have a purpose-built missing value object that we can use in place of NaT, np.nan; this is very tricky though with the current implementation. If everything is EA's then its possible.

Would rather you just patch NaT if you need.
> Would rather you just patch NaT if you need.

NaT not knowing whether it is a datetime or a timedelta is part of the problem; overloading it further would make things worse.

The alternative is to define these methods as regular functions

```
def nat_td_div(left, right):
```

and call the appropriate one in the appropriate place.  By making them into methods we let python figure out which one to call and de-duplicate a lot of code elsewhere.
> The alternative is to define these methods as regular functions

maybe that's better. I agree this is a thorny problem. But we need a comprehensive solution, not more missing data types.

Another option (that involves significantly less code) would be to subclass `Timedelta` and override `__new__` to allow us to get an internal `NaTD` object with `self.value == NPY_NAT`.  The tweaks needed in Timedelta to catch the `if self.value == NPY_NAT` cases would be small compared to the current diff.
how would you reconcile an external pd.NaT with NaTD? is there a way to internally make a NaT_datetime and NaT_timedelta objects so that we always unbox the external object (pd.NaT) for all datetimelike ops ?


> how would you reconcile an external pd.NaT with NaTD?

For the foreseeable future (i.e. until pandas2) I wouldn't expose NaTD to users at all.  So if a user passes pd.NaT, it behaves exactly like it does now.

> is there a way to internally make a NaT_datetime and NaT_timedelta objects so that we always unbox the external object (pd.NaT) for all datetimelike ops ?

The "always" part of that is hard.  Or more specifically, it is hard to do that without risking returning NaTD to users.  The approach in this PR is to identify all the places internally where we would/should use NaTD and swap it in where appropriate.

closing. I think we need a comprehensive soln for this.