REF/API: DatetimeTZDtype
State: closed

A cleanup of DatetimeTZDtype

* Remove magic constructor from string. It seemed dangerous to overload `unit` with either `unit` or a full `datetime64[ns, tz]` string. This required changing `DatetimeTZDtype.construct_from_string` and changing a number of places to use `construct_from_string` rather than the main `__new__`
* Change `__new__` to `__init__` and remove caching. It seemed to be causing more problems that it was worth. You could too easily create nonsense DatetimeTZDtypes like
```
In [3]: t = pd.core.dtypes.dtypes.DatetimeTZDtype(unit=None, tz=None)
```
* Change `.tz` and `.unit` to properties instead of attributes. I've not provided setters. We could in theory, since we're getting rid of caching, but I'd rather wait till there's a demand..

The remaining changes in the DatetimeArray PR will be to

1. Inherit from ExtensionDtype
2. Implement construct_array_type
3. Register the dtype

Labels:
name: Datetime
description: Datetime data dtype
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: Timezones
description: Timezone data dtype

Comments:
Hello @TomAugspurger! Thanks for submitting the PR.

 - There are no PEP8 issues in the file [`pandas/core/arrays/datetimelike.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/core/arrays/datetimelike.py) !

 - There are no PEP8 issues in the file [`pandas/core/dtypes/common.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/core/dtypes/common.py) !

 - There are no PEP8 issues in the file [`pandas/core/dtypes/dtypes.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/core/dtypes/dtypes.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_common.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/tests/dtypes/test_common.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_dtypes.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/tests/dtypes/test_dtypes.py) !

 - There are no PEP8 issues in the file [`pandas/tests/dtypes/test_missing.py`](https://github.com/TomAugspurger/pandas/blob/1ca7fa4fe611ccee04c657eae836049caef18f6c/pandas/tests/dtypes/test_missing.py) !


pls perf test things

caching is essential as these are created quite a lot and they are the same virtually every time
On microbenchmarks, things are fine

master:

```python
In [2]: %time a = pd.core.dtypes.dtypes.DatetimeTZDtype(unit='ns', tz="UTC")
CPU times: user 39 µs, sys: 31 µs, total: 70 µs
Wall time: 75.6 µs

In [3]: %time a = pd.core.dtypes.dtypes.DatetimeTZDtype(unit='ns', tz="UTC")
CPU times: user 15 µs, sys: 0 ns, total: 15 µs
Wall time: 18.8 µs
```

PR:

```python
In [2]: %time a = pd.core.dtypes.dtypes.DatetimeTZDtype(unit='ns', tz="UTC")
CPU times: user 29 µs, sys: 23 µs, total: 52 µs
Wall time: 56 µs

In [3]: %time a = pd.core.dtypes.dtypes.DatetimeTZDtype(unit='ns', tz="UTC")
CPU times: user 16 µs, sys: 0 ns, total: 16 µs
Wall time: 19.8 µs
```

---

ASV for the timeseries, timestamps, and offsets

```
       before           after         ratio
     [6b3490f4]       [982c169a]
-         143±9ms          127±4ms     0.89  
timeseries.DatetimeIndex.time_add_timedelta('tz_aware')
-     2.73±0.05μs      2.45±0.02μs     0.90  timestamp.TimestampOps.time_tz_convert(tzutc())
```

---

Line-profiling reveals that basically all the time is spent on the timezone check.

```

In [5]: %lprun -s -f DatetimeTZDtype.__init__ DatetimeTZDtype(tz='utc')
Timer unit: 1e-06 s

Total time: 4e-05 s
File: /Users/taugspurger/sandbox/pandas/pandas/core/dtypes/dtypes.py
Function: __init__ at line 494

Line #      Hits         Time  Per Hit   % Time  Line Contents
==============================================================
   494                                               def __init__(self, unit="ns", tz=None):
   495                                                   """
   496                                                   An ExtensionDtype for timezone-aware datetime data.
   497
   498                                                   Parameters
   499                                                   ----------
   500                                                   unit : str, default "ns"
   501                                                       The precision of the datetime data. Currently limited
   502                                                       to ``"ns"``.
   503                                                   tz : str, int, or datetime.tzinfo
   504                                                       The timezone.
   505
   506                                                   Raises
   507                                                   ------
   508                                                   pytz.UnknownTimeZoneError
   509                                                       When the requested timezone cannot be found.
   510
   511                                                   Examples
   512                                                   --------
   513                                                   >>> pd.core.dtypes.dtypes.DatetimeTZDtype(tz='UTC')
   514                                                   datetime64[ns, UTC]
   515
   516                                                   >>> pd.core.dtypes.dtypes.DatetimeTZDtype(tz='dateutil/US/Central')
   517                                                   datetime64[ns, tzfile('/usr/share/zoneinfo/US/Central')]
   518                                                   """
   519         1          7.0      7.0     17.5          if isinstance(unit, DatetimeTZDtype):
   520                                                       unit, tz = unit.unit, unit.tz
   521
   522         1          1.0      1.0      2.5          if unit != 'ns':
   523                                                       raise ValueError("DatetimeTZDtype only supports ns units")
   524
   525         1          0.0      0.0      0.0          if tz:
   526         1         27.0     27.0     67.5              tz = timezones.maybe_get_tz(tz)
   527                                                   elif tz is not None:
   528                                                       raise pytz.UnknownTimeZoneError(tz)
   529                                                   elif tz is None:
   530                                                       raise TypeError("A 'tz' is required.")
   531
   532         1          4.0      4.0     10.0          self._unit = unit
   533         1          1.0      1.0      2.5          self._tz = tz
```

---

I switched the properties to `cache_readonly` (which we can do, since we aren't using `_cache` for caching instances now).
Are you worried about memory usage? The time to create a new one from
scratch is identical or faster than looking it up from the cache.

On Thu, Nov 29, 2018 at 7:06 AM Jeff Reback <notifications@github.com>
wrote:

> *@jreback* commented on this pull request.
> ------------------------------
>
> In pandas/core/dtypes/dtypes.py
> <https://github.com/pandas-dev/pandas/pull/23990#discussion_r237478340>:
>
> >      _metadata = ('unit', 'tz')
>      _match = re.compile(r"(datetime64|M8)\[(?P<unit>.+), (?P<tz>.+)\]")
>      _cache = {}
> +    # TODO: restore caching? who cares though? It seems needlessly complex.
> +    # np.dtype('datetime64[ns]') isn't a singleton
>
> try running a good part of the test suite. Its the repeated consruction
> that's a problem, not the single contruction which is fine. W/o caching you
> end up creating a huge number of these
>
> —
> You are receiving this because you modified the open/close state.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/23990#discussion_r237478340>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIuydvXMpt0620svkG4I2c9VNuJfMks5uz9vOgaJpZM4Y42qg>
> .
>

maybe things changed since I created this originally, but w/o caching perf was not great.
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/23990?src=pr&el=h1) Report
> Merging [#23990](https://codecov.io/gh/pandas-dev/pandas/pull/23990?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/08395af43b813b69b1cf038d260104cfeeb3e787?src=pr&el=desc) will **increase** coverage by `49.87%`.
> The diff coverage is `97.95%`.

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

```diff
@@             Coverage Diff             @@
##           master   #23990       +/-   ##
===========================================
+ Coverage   42.38%   92.25%   +49.87%     
===========================================
  Files         161      161               
  Lines       51701    51689       -12     
===========================================
+ Hits        21914    47687    +25773     
+ Misses      29787     4002    -25785
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.66% <97.95%> (?)` | |
| #single | `42.51% <63.26%> (+0.12%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/23990?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `95.62% <ø> (+25.23%)` | :arrow_up: |
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `96.34% <100%> (+45.91%)` | :arrow_up: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `98.43% <100%> (+34.63%)` | :arrow_up: |
| [pandas/core/internals/blocks.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW50ZXJuYWxzL2Jsb2Nrcy5weQ==) | `93.69% <100%> (+41.32%)` | :arrow_up: |
| [pandas/core/dtypes/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL21pc3NpbmcucHk=) | `93.1% <100%> (+35.63%)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `95.33% <97.14%> (+19.17%)` | :arrow_up: |
| [pandas/core/computation/pytables.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tcHV0YXRpb24vcHl0YWJsZXMucHk=) | `92.37% <0%> (+0.3%)` | :arrow_up: |
| [pandas/io/pytables.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3B5dGFibGVzLnB5) | `92.3% <0%> (+0.92%)` | :arrow_up: |
| [pandas/util/\_test\_decorators.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX3Rlc3RfZGVjb3JhdG9ycy5weQ==) | `93.24% <0%> (+4.05%)` | :arrow_up: |
| [pandas/compat/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree#diff-cGFuZGFzL2NvbXBhdC9fX2luaXRfXy5weQ==) | `58.36% <0%> (+8.17%)` | :arrow_up: |
| ... and [125 more](https://codecov.io/gh/pandas-dev/pandas/pull/23990/diff?src=pr&el=tree-more) | |

------

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

I've removed `_coerce_to_dtype`.
dont you need to remove the ``_coerce_to_dtype`` tests?
https://github.com/pandas-dev/pandas/pull/23990/commits/7ab2a74c304c6791f8f28edc01b7f43b2cbd2fab removed all the `_coerce_to_dtype` tests.
I may be doing something wrong, but it seems like memory usage isn't increasing either.

```python
from memory_profiler import profile
import pandas as pd


@profile(precision=4)
def check():
    dts = [pd.core.dtypes.dtypes.DatetimeTZDtype('ns', 'UTC')] * 100_000_000
    return dts
```

master: ![master](https://user-images.githubusercontent.com/1312546/49298301-a078d780-f482-11e8-9ac9-963dad296d60.png)


pr: ![pr](https://user-images.githubusercontent.com/1312546/49298302-a078d780-f482-11e8-9a63-4db09cbc7ead.png)

But the chart for master doesn't look right. Shouldn't that be linear in the number of DatetimeTZDtypes we create? Perhaps it is correctly, and we're just not reporting our memory usage correctly?
Although memory_profiler uses psutil, which I don't *think* relies on `__sizeof__`, though I may be wrong.
I'll revert the magic construction changes with a futurewarning. But DatetimeTZDtype isn't part of the public API. Dask, at least, doesn't use it directly.
Do we typically add release notes for something like this (deprecations that aren't part of the public API, but are possibly in use downstream)?
Added to the deprecation log.
> Do we typically add release notes for something like this (deprecations that aren't part of the public API, but are possibly in use downstream)?

this IS certainly part of the public API as ``DatetimeTZDtype`` is public.
I *know* ``pyarrow`` uses this at the very least. (I don't know if the 2 form of the construction is used though). cc @wesm 
Ah, I didn't realize it was exported in `api.types`, it's not in the API docs. I'll add it to api.rst at the top-level (as of https://github.com/pandas-dev/pandas/pull/23581)
Added a release note. Going to followup on api.rst later, since we don't have an `array` section for datetime array there yet.
thanks @TomAugspurger lgtm. let's merge on green. can you add a ref into #6581 
@datapythonista any idea what's up with the azure CI? https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=4597. Seems to have hit https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=4596 too.
Haven't seen those errors before, I guess something is failing in their side. @DavidStaheli do you mind taking a look at https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=4597
@TomAugspurger needs a rebase as well
@TomAugspurger I was trying to create an incident for azure, but the url does some weird redirect with the language and it crashes. Not sure if it's Linux specific, can you check if the "Create incident" button works for you: https://azure.microsoft.com/en-us/support/create-ticket/ (the Basic technical support one)

Or @vtbassmatt, may be you can help with the above? See the error in https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=4597, we are getting that often
I was checking, and numba is getting the same error from azure. I guess something is down for the whole azure-pipelines.

https://dev.azure.com/numba/numba/_build/results?buildId=471
I suspect it's a little bit of abuse fighting that was turned up a little too high. I think we've addressed it now.
thanks @TomAugspurger