API: Tick arithmetic respects calendar time
State: closed

- [x] closes #20633
- [ ] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

This PR changes `Tick` arithmetic to respect calendar time akin to `Datoffset` for `Timestamp` and `DatetimIndex` (and hence `Series`).  There may be some embedded Tick work-around code (I am thinking in `DatetimeIndex._generate` and `resample` that may be refactorable after this change.

Labels:
name: API Design
name: Frequency
description: DateOffsets
name: Timezones
description: Timezone data dtype

Comments:
Definitely +1 on doing this for Day, but it is less clear what this means for the other Tick subclasses.  What's the motivation?
For the other Ticks, it's mostly to keep them consistent with the definition of `Day` e.g. a user should expect 24 `Hour`s to behave similarly as 1 `Day`. Otherwise the other Tick subclasses would be redundant with Timedeltas as well.
> For the other Ticks, it's mostly to keep them consistent with the definition of Day e.g. a user should expect 24 Hours to behave similarly as 1 Day. Otherwise the other Tick subclasses would be redundant with Timedeltas as well.

I can see some usefulness here, but what about use cases where the frequency _should_ be Timedelta-like?  e.g. if I call `pd.date_range('2016-01-02 03:04', periods=1000, freq='H', tz='US/Pacific')`, I'd be surprised if the resulting index skipped an hour at a DST change.

This is also going to affect `timedeltas.delta_to_nanoseconds`, I'll have to look at this more closely.
@jbrockmendel 

I think the core confusion for users is the inherent ambiguity when specifying a string frequency like `'D'` on whether the strings mean calendar time or absolute time.

Pandas advertises them as (Date)offsets, but there's an implicit (and not-clearly-documented) consistency throughout the API that string offsets passed to other methods like `resample` or `date_range` respect absolute time which is consistent with `pd.tseries.frequencies.to_offset('D')` returning a `Day` Tick that also respects absolute time. (This is probably why we have a lot of kludgey code dancing around `Day`)

That being said, I would love for all the frequency strings to map to calendar-respecting `Ticks`
/`DateOffsets`, but that would API breaking for `df.resample('D')` with timezones.

For the sake of consistency, I am okay with the API break, but interested to get your initial thoughts
Probably should open this discussion up to other too.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22195?src=pr&el=h1) Report
> Merging [#22195](https://codecov.io/gh/pandas-dev/pandas/pull/22195?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/776fed3ab63d74ddef6e5af1a702b10c2a30bbb6?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/22195/graphs/tree.svg?height=150&src=pr&token=eZ4WkYLtcO&width=650)](https://codecov.io/gh/pandas-dev/pandas/pull/22195?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #22195      +/-   ##
==========================================
+ Coverage   92.06%   92.06%   +<.01%     
==========================================
  Files         169      169              
  Lines       50694    50697       +3     
==========================================
+ Hits        46671    46674       +3     
  Misses       4023     4023
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.47% <100%> (ø)` | :arrow_up: |
| #single | `42.32% <12.5%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22195?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/22195/diff?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `97.16% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/22195/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `95.49% <100%> (-0.02%)` | :arrow_down: |

------

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

@jbrockmendel After thinking about it; I am convinced to only change `Day` to respect calendar arithmetic and leaving the rest of the ticks alone.

To do this I was thinking of having `Day` subclass `DateOffset` instead of `Tick` because the arithmetic paths are already determined by checking for `DateOffset` vs `Tick`. Any thoughts/reservations?
@mroeschke ok with your suggestion, it *might* make some code simpler (though likely to break some tests I think)
I think we're on the same page.  I think we should make `Day` (and with it "D") correspond to calendar time like this PR does.

We also need to retain a way to specify `timedelta`-like frequencies, which is why I think we should keep `Hour`, `Minute`, ..., `Nano` as they are.  Then a user can specify an absolute-day as `"24H"`.  Conversely they can specify "every day at noon and midnight" as `".5D"`.

There is also an issue (not specific to this PR, but closely related) of how to handle ambiguous/nonexistent times.  If I start at `ts = pd.Timestamp("2018-11-03 01:00:00", tz='US/Pacific')` and add a calendar-Day offset, do we want `Timestamp('2018-11-04 01:00:00-0700', tz='US/Pacific')` or `Timestamp('2018-11-04 01:00:00-0800', tz='US/Pacific')`?

Conversely, If I start at `ts = pd.Timestamp("2019-03-09 02:00:00", tz='US/Pacific')` and add a calendar-Day, do we raise NonExistentTimeError or allow some fudge-factor?

Thanks for the input @jreback and @jbrockmendel:

In regards to the `Timestamp + Day = Ambiguous Datetime` issue, I am partial toward raising an `AmbiguousTimeError` for "fall back" DST behavior and `NonExistentTimeError` for "spring forward" behavior for now. In the future we can consider adding a special kwarg to `Day.apply` to handle these ambiguous times.
Closing as this will require a different implimentation