ENH/BUG: Add is_dst method to DatetimeIndex and Timestamp to solve AmbiguousTimeError
State: closed

- [x] closes #18885
- [x] closes #18946
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

The issues above required a new method to keep track if each timestamp in the `DatetimeIndex` was previously in daylight savings time in order to pass into `tz_localize(..., ambiguous=...)`

Therefore, added a `is_dst` method to `DatetimeIndex` and `Timestamp`.

Labels:
name: Bug
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Timezones
description: Timezone data dtype

Comments:
Is this related to the “fold” arg added in py3?
> Is this related to the “fold” arg added in py3?

Yup fairly similar. The `ambiguous=True/False` argument is essentially similar to the `fold=0/1` argument in py3. 

These `AmbiguousTimeError`s stem from internal routines that transform a timestamp to an ambiguous time. I think it's best we try to return a sensible time based on the original timeseries instead of leaving the user with an error they cannot control. 
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/22560?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@a5fe9cf`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff            @@
##             master   #22560   +/-   ##
=========================================
  Coverage          ?   92.04%           
=========================================
  Files             ?      169           
  Lines             ?    50789           
  Branches          ?        0           
=========================================
  Hits              ?    46747           
  Misses            ?     4042           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.45% <100%> (?)` | |
| #single | `42.29% <33.33%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22560?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/22560/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `97.77% <100%> (ø)` | |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/22560/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.56% <100%> (ø)` | |

------

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

Ready for another look @jbrockmendel when you get the chance.
Hello @mroeschke! Thanks for updating the PR.

 - There are no PEP8 issues in the file [`pandas/core/indexes/datetimelike.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/core/indexes/datetimelike.py) !

 - There are no PEP8 issues in the file [`pandas/core/indexes/datetimes.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/core/indexes/datetimes.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/datetimes/test_timezones.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/tests/indexes/datetimes/test_timezones.py) !

 - There are no PEP8 issues in the file [`pandas/tests/reshape/merge/test_merge.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/tests/reshape/merge/test_merge.py) !

 - There are no PEP8 issues in the file [`pandas/tests/scalar/test_nat.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/tests/scalar/test_nat.py) !

 - There are no PEP8 issues in the file [`pandas/tests/scalar/timestamp/test_timezones.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/tests/scalar/timestamp/test_timezones.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_datetime_values.py`](https://github.com/mroeschke/pandas/blob/c03792e22c10a1affdfde6c119c728b4451f1434/pandas/tests/series/test_datetime_values.py) !


I would recommend against adding an `is_dst` method. If this is for resolving ambiguous times, it would be preferable to use something more akin to a `fold` method, since there are several situations where an ambiguity cannot be resolved by querying DST or not (base offset shifts).

In general DST/not DST is only a proxy for the real information you want in all cases I can think of - the offset, the tzname or which side of an ambiguity you are on. All of those can be independently queried and/or specified without mentioning DST.
> These AmbiguousTimeErrors stem from internal routines that transform a timestamp to an ambiguous time. 

I'm not clear on what problem this is solving.  Can you give an example of the internal routines that produce ambiguous times?

@pganssle and I don't always see eye-to-eye on how to treat public vs private attributes, but in general if he's got a strong opinion, it is worth listening to.
I also wanted to mention that we deliberately added an `ambiguous` keyword and deprecated `is/infer_dst` because not all ambiguities are DST issues, if I remember well (https://github.com/pandas-dev/pandas/pull/7963, there was some discussion about it, but I am not very familiar with timezone handling details), so we should maybe consider that issue here as well. But I see that @pganssle in the meantime already noted something similar :)
@jbrockmendel 
> Can you give an example of the internal routines that produce ambiguous times?

When rounding a tz-aware date time to a particular frequency, essentially the steps are:

* convert the date to a naive timestamp
* round the i8 value to the frequency
* relocalize the i8 value to the timezone

You can't necessarily convert to UTC first because it can affect the rounding (unless I am missing something).

So in #18946, a date that sits on the DST border `2017-10-29 02:00:00+02:00` that's rounded to an hour (`'H'`):

* Convert to `2017-10-29 02:00:00`
* Round to the nearest hour (`2017-10-29 02:00:00`)
* Relocalize to `2017-10-29 02:00:00+02:00` or `2017-10-29 02:00:00+01:00`?

I guess #18946 can be solved by added an `ambiguous` kwarg to `round` also. However the routine above is the cause for other issues as well #18885, but that _may_ be solved by converting to UTC first.

Using DST information may be a too hand-wavy solution to solve both #18946 and #18885. At minimum, I had also though having a `is_dst` would be a nice-to-have enhancement for DatetimeIndexes.
@mroeschke I'm guessing that rounding issues in UTC is mainly at the day-or-higher level, right? Since rounding to the nearest minute or hour will cause no problems.

I think the correct solution here is to introduce the `fold` concept into pandas, and start deprecating `pytz`-isms.

I see a few good ways forward:

1. Write `fold` compatible wrappers for `pytz` and start deprecating `pytz`-isms like `localize` and `normalize`.
2. Write wrappers for `dateutil` time zones that expose some `pytz`-isms (which you can deprecate), and stop using `pytz` entirely.
3. Implement a third time zone library optimized for array types and drop both `pytz` and `dateutil`.

In all cases, I think `fold` semantics is the best way to go.

I am very strongly hoping that within a year I'll be able to get a fast, compiled Rust or C backend for many of `dateutil`'s modules, including `tz`, which should make it a lot cheaper to make bulk function calls (particularly if I expose a C API or similar so you don't have any Python function call overhead), but number 3 might still be the best option since it's likely that supporting something that would work *really* well for array types in `dateutil` *and* work really well for normal Python implementations would be more complicated than just maintaining two separate libraries.
> You can't necessarily convert to UTC first because it can affect the rounding (unless I am missing something).

I think there may be an implicit assumption in here that should be made explicit.  Does the user want the rounding to be based on wall time or actual time?  (My intuition is actual time)

```
ts = pd.Timestamp('2017-10-29 02:59:00+0200', tz='Europe/Madrid')
m = pd.Timedelta(minutes=1)

>>> ts + m
Timestamp('2017-10-29 02:00:00+0100', tz='Europe/Madrid')
>>> ts - 59*m
Timestamp('2017-10-29 02:00:00+0200', tz='Europe/Madrid')
```

If I were rounding `ts` to the nearest hour, it seems clear I'd want the one 1 minute away rather than 59 minutes away, despite the fact they have the same difference in wall time.

Is this intuition not applicable to the case you're considering?
@jbrockmendel I think for time scales of hours or less it's probably actual time, but time scale of days they almost certainly want wall time.

Imagine rounding to midnight UTC, then going back to PST/PDT. No one wants to round to 17:00 sometimes and 16:00 others.

To be honest it's probably always wall time, but on sufficiently small time scales the two are equivalent.
I agree with @pganssle and would imagine users would want to round based on wall time in general.

Thanks for the feedback guys. I'm convinced I need to surface a `fold` (or `ambiguous` in pandas terms) argument in `round` then. Closing.