Update type for PeriodDtype / DatetimeTZDtype / IntervalDtype
State: closed

Split from PeriodArray.

Updating the type to be the more correct `Period`.

Also removed the essentially unused `IntervalDtypeType`. If this doesn't break anything, it seems like there are decent chunks of `_get_dtype_type` that aren't being hit, aside from `test__get_dtype_type`.


Labels:
name: Dtype Conversions
description: Unexpected or buggy dtype conversions
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

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

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

 - There are no PEP8 issues in the file [`pandas/core/dtypes/dtypes.py`](https://github.com/TomAugspurger/pandas/blob/8a8bdb05008ed52fe0eb8d4168347bc2af116b4a/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/8a8bdb05008ed52fe0eb8d4168347bc2af116b4a/pandas/tests/dtypes/test_common.py) !


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

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

```diff
@@            Coverage Diff             @@
##           master   #22938      +/-   ##
==========================================
- Coverage   92.19%   92.19%   -0.01%     
==========================================
  Files         169      169              
  Lines       50822    50820       -2     
==========================================
- Hits        46855    46852       -3     
- Misses       3967     3968       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.6% <93.75%> (-0.01%)` | :arrow_down: |
| #single | `42.36% <68.75%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/22938?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/dtypes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/22938/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Jhc2UucHk=) | `100% <ø> (ø)` | :arrow_up: |
| [pandas/core/dtypes/common.py](https://codecov.io/gh/pandas-dev/pandas/pull/22938/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbW1vbi5weQ==) | `95.03% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/22938/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `95.82% <87.5%> (-0.3%)` | :arrow_down: |

------

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

Something that just occurred to me: We have types where "missing" is represented by a value that isn't an instance of the type. e.g. `NaT`, the missing value for Datetime and Period, isn't an instance of Timestamp or Period. Interval uses `np.nan`.

Should the interface define that a missing value for a dtype be an instance of the type? It's not clear to me that it should. Are people OK with saying that NA values are allowed to be other types?
> Should the interface define that a missing value for a dtype be an instance of the type? It's not clear to me that it should. Are people OK with saying that NA values are allowed to be other types?

Well, currently I think almost none of our actual EAs have an `na_value` of their type, so I think that clearly answers the question? 
I also don't see how it is possible to have this in general, as you don't always have control over the scalar type.
this change still seems wrong

e.g.

```
In [1]: np.dtype('float64').type
Out[1]: numpy.float64

In [2]: type(np.dtype('float64').type)
Out[2]: type
```

is what we are emulating here. This is a wholesale change of this. I am not sure why you actually need to do this. Having a concrete type (e.g. a Period) rather a type(class), e.g. the type of the class is semantically correct here. I cannot believe this doesn't break anything.
Your code shows that this PR is doing the right thing.

```python
In [3]: np.array([1, 2]).dtype.type
Out[3]: numpy.int64

In [4]: type(np.array([1, 2]).dtype.type)
Out[4]: type

In [6]: pd.core.arrays.period.PeriodArray([1, 2], 'D').dtype.type
Out[6]: pandas._libs.tslibs.period.Period

In [7]: type(pd.core.arrays.period.PeriodArray([1, 2], 'D').dtype.type)
Out[7]: type

```
why do you need to change at all? what exactly is breaking on master?
It's incorrect according to the definition of an ExtensionArray. I'm
updating our internal arrays to implement the interface, and this is a
standalone change that can be easily reviewed and approved since it's a
simple, uncontroversial change.

On Tue, Oct 2, 2018 at 3:45 PM Jeff Reback <notifications@github.com> wrote:

> why do you need to change at all? what exactly is breaking on master?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22938#issuecomment-426423833>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIg4vey1pQWOdXD80oXL7EvuhgrI3ks5ug9CGgaJpZM4XDOmT>
> .
>

> I cannot believe this doesn't break anything.

Because we actually never use this / rely on this. Or at least, only use `dtype.type` when `.dtype` is a numpy dtype.

We discussed before that the `ExtensionDtype.type` was more "pro forma" and not really doing anything (at the moment).

But if we keep it, I agree with Tom that we should at least have our internal EA dtypes to be consistent with the interface (which is what this PR is doing).
so i am not against changing for code cleanliness and maybe i am pounding sand here

but this makes EA dtypes inconsistent with numpy dtypes 

now it probably doesn’t make any practical difference but maybe let’s change the text that they are (not a real numpy dtype)

they will now not even pretend to be a duck like numpy dtype at all
> but this makes EA dtypes inconsistent with numpy dtypes

I think that's the misunderstanding. This is making it consistent with
NumPy. The dtype.type should be the scalar type (int, float, Period,
Timestamp).

On Tue, Oct 2, 2018 at 5:55 PM Jeff Reback <notifications@github.com> wrote:

> so i am not against changing for code cleanliness and maybe i am pounding
> sand here
>
> but this makes EA dtypes inconsistent with numpy dtypes
>
> now it probably doesn’t make any practical difference but maybe let’s
> change the text that they are (not a real numpy dtype)
>
> they will now not even pretend to be a duck like numpy dtype at all
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/22938#issuecomment-426457406>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIkaIG11j51aOS6v92wjWP9pQahctks5ug-7wgaJpZM4XDOmT>
> .
>

> but this makes EA dtypes inconsistent with numpy dtypes

@jreback does 
https://github.com/pandas-dev/pandas/pull/22938#issuecomment-426402397 make sense? On master we're inconsistent with numpy, since for these types `.type` isn't the scalar type of the array.

In code that's

```python
In [2]: arr = np.array([1, 2])

In [3]: assert type(arr[0]) == arr.dtype.type

In [4]: parr = pd.PeriodIndex([2000, 2001], freq="D")

In [5]: assert type(parr[0]) == parr.dtype.type
---------------------------------------------------------------------------
AssertionError                            Traceback (most recent call last)
<ipython-input-5-2e0cefc2b29e> in <module>
----> 1 assert type(parr[0]) == parr.dtype.type

AssertionError:

In [6]: type(parr[0]), parr.dtype.type
Out[6]: (pandas._libs.tslibs.period.Period, pandas.core.dtypes.dtypes.PeriodDtypeType)
```
Only failure here is the S3 failure, which was hopefully resolved on master.