REF: Make PeriodArray an ExtensionArray
State: closed

Closes #22862 
Closes #14108
Closes #18053

Labels:
name: Period
description: Period data type
name: ExtensionArray
description: Extending pandas with custom dtypes or arrays.

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

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

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

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

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

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

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

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

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

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

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

 - There are no PEP8 issues in the file [`pandas/core/indexes/accessors.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/core/indexes/accessors.py) !

 - There are no PEP8 issues in the file [`pandas/core/indexes/base.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/core/indexes/base.py) !

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

 - There are no PEP8 issues in the file [`pandas/core/indexes/period.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/core/indexes/period.py) !

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

 - There are no PEP8 issues in the file [`pandas/io/packers.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/io/packers.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arithmetic/test_period.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/arithmetic/test_period.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arrays/test_datetimelike.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/arrays/test_datetimelike.py) !

 - There are no PEP8 issues in the file [`pandas/tests/arrays/test_period.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/arrays/test_period.py) !

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

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

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

 - There are no PEP8 issues in the file [`pandas/tests/extension/test_integer.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/extension/test_integer.py) !

 - There are no PEP8 issues in the file [`pandas/tests/extension/test_period.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/extension/test_period.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_combine_concat.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/frame/test_combine_concat.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_constructors.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/frame/test_constructors.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_operators.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/frame/test_operators.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_replace.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/frame/test_replace.py) !

 - There are no PEP8 issues in the file [`pandas/tests/frame/test_reshape.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/frame/test_reshape.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/common.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/common.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_astype.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_astype.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_construction.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_construction.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_formats.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_formats.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_indexing.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_indexing.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_ops.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_ops.py) !

 - There are no PEP8 issues in the file [`pandas/tests/indexes/period/test_tools.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/indexes/period/test_tools.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/formats/test_format.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/io/formats/test_format.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/test_feather.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/io/test_feather.py) !

 - There are no PEP8 issues in the file [`pandas/tests/io/test_parquet.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/io/test_parquet.py) !

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

 - There are no PEP8 issues in the file [`pandas/tests/reshape/test_concat.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/reshape/test_concat.py) !

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

 - There are no PEP8 issues in the file [`pandas/tests/series/test_api.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/series/test_api.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_apply.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/series/test_apply.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_constructors.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/series/test_constructors.py) !

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

 - There are no PEP8 issues in the file [`pandas/tests/series/test_operators.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/series/test_operators.py) !

 - There are no PEP8 issues in the file [`pandas/tests/series/test_period.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/series/test_period.py) !

 - There are no PEP8 issues in the file [`pandas/tests/test_base.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/tests/test_base.py) !

 - There are no PEP8 issues in the file [`pandas/util/testing.py`](https://github.com/TomAugspurger/pandas/blob/5462bd7c46b93e79b768cdedef38ea165d608a2f/pandas/util/testing.py) !

 - Complete extra results for this file :

> file_to_check.py:2537:-258: W605 invalid escape sequence '\('
file_to_check.py:2537:-255: W605 invalid escape sequence '\)'
---



##### Comment last updated on October 20, 2018 at 01:57 Hours UTC
Many things are broken right now.
> the reason you mention this is to not have the confusion with BlockManager _data of DataFrame/Series ?

I was thinking of #19658
Still haven't simplified, still not worth looking at.
Updated. Hopefully we're down to a single failing test, `pandas/tests/scalar/period/test_period.py::TestMethods::test_add_timestamp_raises`

The problem there is in `distpatch_to_extension_ops`. We need to avoid turning either into a list. https://github.com/pandas-dev/pandas/issues/22922
FYI, a biggish kinda-breaking change is that that the following are now different

```python
In [1]: import pandas as pd

In [2]: pd.Series([pd.Period("2000", "A"), pd.Period('2001', 'A')])
Out[2]:
0    2000
1    2001
dtype: object

In [3]: pd.Series(pd.core.arrays.period_array([pd.Period("2000", "A"), pd.Period('2001', 'A')]))
Out[3]:
0    2000
1    2001
dtype: period[A-DEC]
```

Previously, you could use a `.dt.<foo>` on `Out[2]`. We could add back that ability. I think we should do it with a deprecation warning.
would we not infer [1] to Period ?
> would we not infer [1] to Period ?

We don't in Series, we do in Index. I could look into what it would take to support that.
So in `Index.__new__` we do a `lib.infer_dtype(values)`. We would need something similar in the series constructor, maybe somewhere in `_sanitize_array`. Not sure where is best yet.
yeah thought we did some inference on object dtypes 
> We don't in Series, we do in Index. I could look into what it would take to support that.

The more we can do to align the behavior between these constructors (not necessarily this PR) the better off we'll be.  We could go so far as to have EA([...]) perform inference similarly and share tests the same way we do in tests/arithmetic.
Are the PeriodDType changes meaningful independently of the rest of this PR?  If so, splitting that out seems like a good way to make real progress, hopefully without racking up more merge conflicts.
Oh dang, woops.  I was trying to make a PR on your PR to rebase, but somehow pushed directly to this PR.  Until instructed otherwise, I'm going to avoid touching anything for fear of making things worse.
No worries. I can force push my local version tomorrow if needed.

________________________________
From: jbrockmendel <notifications@github.com>
Sent: Sunday, October 14, 2018 6:21:01 PM
To: pandas-dev/pandas
Cc: Tom Augspurger; Mention
Subject: Re: [pandas-dev/pandas] [WIP] PeriodArray (#22862)


Oh dang, woops. I was trying to make a PR on your PR to rebase, but somehow pushed directly to this PR. Until instructed otherwise, I'm going to avoid touching anything for fear of making things worse.

—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub<https://github.com/pandas-dev/pandas/pull/22862#issuecomment-429672044>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ABQHIkw01ZpUCUVwF4Le1MHj2yNGOv63ks5uk8bdgaJpZM4W9VVP>.

> Are the PeriodDType changes meaningful independently of the rest of this PR?

I *think* they are tied to becoming an actual ExtensionArray / ExtensionDtype, but not fully sure
> I think for PeriodArray-only internal implementations, we should maybe mainly be using `_data`?

Can we use something other than `_data`?  How about something really explicit like `_i8values` or `_initvals`?
@jorisvandenbossche @TomAugspurger A few thoughts since an early-morning call didn't work out.

My view is increasingly that we should try to get all of Datetime/Timedelta/Period Arrays across the finish line at the same time.  That way we can get an all-at-once transition to "`.values` is lossless".

Things that can be shared in DatetimeLikeArrayMixin rather than PeriodArray should be shared (e.g. #23159)

The new thing I'm concerned about is caching.  ATM the Index subclasses override several of the EA properties with cache_readonly (think `_isnan`, which is used by arithmetic ops).  When moving to composition, we'll want to somehow trick EA's backing Indexes into caching.
> The new thing I'm concerned about is caching. [...] we'll want to somehow trick EA's backing Indexes into caching.

I'm not sure I follow. The EAs are mutable, so they can't be cached. Once an array (ndarray or EA) is inside an Index, we assume (but don't check) that the values aren't changed.

```python
In [6]: arr = np.array([1., 2., 3.])

In [7]: idx = pd.Index(arr, copy=False)

In [8]: idx.hasnans
Out[8]: False

In [9]: arr[0] = np.nan

In [10]: idx
Out[10]: Float64Index([nan, 2.0, 3.0], dtype='float64')

In [11]: idx.hasnans
Out[11]: False
```
@jbrockmendel are you on gitter?

> My view is increasingly that we should try to get all of Datetime/Timedelta/Period Arrays across the finish line at the same time.

To be clear, the alternative is what I have been doing here: a bunch of `TODO(datetimearray): remove this` when everything is done. 
> My view is increasingly that we should try to get all of Datetime/Timedelta/Period Arrays across the finish line at the same time. 

But that would then even give a bigger PR in the end than this one already is? 
Of course you can move things out of that final PR, but not everything. Eg in the take/concat PR, you are moving something that is then not even used in the PR, because it will only be used once we actually do the split. So I am not sure I find that a good thing.
Regarding caching: it is indeed a similar problem as with Index backed by numpy array. 

But one thing I think @jbrockmendel is pointing to, is that some of the arithmetic / other implementations are currently using cached attributes, so if they move to the Array class (where we won't use cached attributes), that means loosing some of this benefit of caching. 

For me that would be an acceptable downside of moving arithmetic / type-specific implementations to the Arrays.
> For me that would be an acceptable downside of moving arithmetic / type-specific implementations to the Arrays.

well no this is not acceptable
theee routines like null checking are used extensively and this would be a perf hit virtually evrywhere in pandas
We have the same performance hit for floats? 

So what do you propose as an alternative? 
The only alternative I can think of, is implement caching on the datetimelike Array classes, and then deal with invalidating the cache in things like `__setitem__` or any other method that changes the array.
> But that would then even give a bigger PR in the end than this one already is?

That's why I've been doing it in small pieces.
> I'm not sure I follow. The EAs are mutable, so they can't be cached

Right, that's the "somehow trick" part of the problem.
> So what do you propose as an alternative?

Well, inheritance would solve the problem.  Heck, we could do inheritance _and_ composition, have PeriodIndex.values return a PeriodArray
OK, I'm reasonably happy with the PeriodArray constructors now and their uses now. To summarize,

- `period_array` is pretty flexible. Does everything `PeriodIndex.__new__`, except for range generation. There are aspects I dislike (like the handling of floats) but it's reasonable.
- _`from_sequence(cls, scalars: Sequence[Period], dtype: PeriodDtype, copy=False)`
- `_from_factorized(cls, values: ndarray[int], PeriodArray)`
- `_from_datetime64(cls, values: ndarray[datetime64[ns]], freq: Freq, tz: Optional[tzinfo])`
- `PeriodArray.__init__(values: Union[Series[period], Index[period], PeriodArray, ndarray[int]], freq: Union[str, Tick])` 

In particular, `PeriodArray.__init__` is always fast. It can always be done with 0 copy and 0 inference. I like that property.

I wouldn't recommend reviewing yet, aside from a glance at how the constructors are used in `arrays/period.py` and `indexes/period.py`. I'd still like to clean things up a bit more before taking anyone's time.

I'm probably going to move on to the blockers listed in #23185 now. I've added xfails for those blockers.
That summary is useful, thanks.  I'm looking forward to re-reviewing after pieces are split off.  Dumb but relevant reason: my ancient laptop struggles to load the full diff.
> I wouldn't recommend reviewing yet, aside from a glance at how the constructors are used in arrays/period.py and indexes/period.py. I'd still like to clean things up a bit more before taking anyone's time.

What aspects do you consider not ready? Or what are the things you want to further change? (apart from getting the tests pass)

The constructors look good to me. Is there a reason you kept `_simple_new` as an alias?