API: return Index instead of array from DatetimeIndex field accessors (GH15022)
State: closed

 - [x] closes #15022
 - [x] tests added / passed
 - [x] passes ``git diff upstream/master | flake8 --diff``
 - [x] whatsnew entry

This changes the datetime field accessors of a DatetimeIndex (and PeriodIndex, etc) to return an Index object instead of a plain array:

So for example:
```
# PR

In [1]: idx = pd.date_range("2015-01-01", periods=5, freq='10H')

In [2]: idx
Out[2]: 
DatetimeIndex(['2015-01-01 00:00:00', '2015-01-01 10:00:00',
               '2015-01-01 20:00:00', '2015-01-02 06:00:00',
               '2015-01-02 16:00:00'],
              dtype='datetime64[ns]', freq='10H')

In [3]: idx.hour
Out[3]: Int64Index([0, 10, 20, 6, 16], dtype='int64')

```

instead of

```
# master

In [1]: idx = pd.date_range("2015-01-01", periods=5, freq='10H')

In [2]: idx.hour
Out[2]: array([ 0, 10, 20,  6, 16], dtype=int32)
```

Labels:
name: Datetime
description: Datetime data dtype
name: API Design

Comments:
One failing test I am not sure what to do about. On master, the following preserves the name:

```
In [1]: idx = pd.date_range("2015-01-01", periods=10, freq='10H', name='name!')

In [2]: idx.map(lambda x: x.hour)
Out[2]: Int64Index([0, 10, 20, 6, 16, 2, 12, 22, 8, 18], dtype='int64', name='name!')
```

but now not anymore. The reason for this is the `DatetimeIndex.map` implementation, where if the function returns an Index, this is returned, but otherwise `Index.map` is used and this passes through the attributes. 
There actually also seems to be a bug in the map implementation: https://github.com/pandas-dev/pandas/blob/5067708f0199a0b614586dbbc1a1536fa4442b65/pandas/tseries/base.py#L331-L343 (line 337 the `shallow_copy` is called, but nothing is done with the result) cc @nateyoder 
Maybe also more in general: should those field accessors preserve the index name?
these should have the name of the ``Series`` themselves (e.g. the name of the values)
> these should have the name of the Series themselves

There is no Series here, it is only about Index
> Maybe also more in general: should those field accessors preserve the index name?

yes
obviously when finished, need a sub-section in whatsnew for this, it is technically an API change, though actually should be back-compat
> these should have the name of the Series themselves
> There is no Series here, it is only about Index

no what I mean is the name of the result index should be the name of the original Series values

IOW


```
In [16]: s = Series(pd.date_range('20130101',periods=3), name='foo')

In [17]: s.dt.day
Out[17]: 
0    1
1    2
2    3
Name: foo, dtype: int64

In [18]: Index(s.dt.day, name='foo')
Out[18]: Int64Index([1, 2, 3], dtype='int64', name='foo')
```
Sorry, I still don't understand. Do you mean that eg the `s.index.day` attribute takes `s.name` as its name (instead of `s.index.name`)? 
But and `Index` can live completely independent of a Series, so I don't see why this should be the case (or do we have examples of that somewhere else in pandas?)
> Sorry, I still don't understand. Do you mean that eg the s.index.day attribute takes s.name as its name (instead of s.index.name)?
But and Index can live completely independent of a Series, so I don't see why this should be the case (or do we have examples of that somewhere else in pandas?)


yes of course, you are working on the ``values``, so you return the ``values`` ``.name`` attribute. this is standard practice, for example any type of operation.

This is de-facto the same as doing.

```
In [1]: s = Series([1,2,3],index=Index(list('abc'), name='bar'), name='foo')

In [2]: s
Out[2]: 
bar
a    1
b    2
c    3
Name: foo, dtype: int64

In [3]: pd.Index(s)
Out[3]: Int64Index([1, 2, 3], dtype='int64', name='foo')
```
@jreback the starting object in this PR is an *index*, not a series. So the values I pass to `Index` are coming from an Index, not from a Series. 
So I suppose it has to take the name of the Index, but there is no Series involved here.

(`self.name` is the Index name)
@jorisvandenbossche this *only* affects the delegates (which is *always* a Series). NOT directly from the index (though actually that *should* also propogate the name).
@jorisvandenbossche  - I don't feel strongly about this, but given that the dt accessors return a like shaped array, wouldn't it make sense to wrap the results back in a `Series`?  E.g., no different than this:

```python
In [35]: s = pd.Series(['a', 'b', 'c'])

In [36]: s.str.upper()
Out[36]: 
0    A
1    B
2    C
dtype: object

```
@chris-b1 this PR is about Index, not Series (will add better description at the top and whatsnew to make this more clear). So the equivalent example is:

```
In [55]: s = pd.Index(['a', 'b', 'c'])

In [56]: s.str.upper()
Out[56]: Index(['A', 'B', 'C'], dtype='object')
```

So in fact I make the datetime fields more consistent with the the str methods, as the first now return an array, while the string methods already return the result wrapped in an Index.
Oh, yep that makes sense then, sorry I basically only read the title.
Ah, yes :-) updated the title to make that more clear (although it is not only for DatetimeIndex, but also PeriodIndex and TimedeltaIndex). And that reminds me, I don't think I already changed this for TimedeltaIndex

TODO:

- [x] same change for TimedeltaIndex field accessors for consistency? (-> days, seconds, total_seconds)
> same change for TimedeltaIndex field accessors for consistency? (-> days, seconds, total_seconds)

this should be for *all datetime-like* accessors I think (no exclusions).

> >  same change for TimedeltaIndex field accessors for consistency? (-> days, seconds, total_seconds)

> this should be for all datetime-like accessors I think (no exclusions).

Yep, I did that, but also noticed that there are quite some other operations on Index objects that also return an array. Maybe we should have a more general discussion on where to draw the line (but that is for another issue)
> Yep, I did that, but also noticed that there are quite some other operations on Index objects that also return an array. Maybe we should have a more general discussion on where to draw the line (but that is for another issue)

yes pls create an issue (maybe with checkboxes)?
@jreback the failing test is one where a boolean Series is now object dtyped. This is because we don't have a boolean index, so it gets object dtype. But if it is then converted to a Series, this keeps object dtype

```
In [24]: idx = pd.date_range("2015-01-01", periods=5, freq='10H')

In [25]: idx.is_month_start
Out[25]: Index([True, True, True, False, False], dtype='object')

In [26]: pd.Series(idx).dt.is_month_start
Out[26]: 
0     True
1     True
2     True
3    False
4    False
dtype: object
```

Is there a good way to deal with this? (I can infer the dtype when it is object within the Properties delegator)

That is actually a side effect of this PR I did not consider. Returning an object index with booleans is not really good .. 

On second thoughts, it is actually totally not acceptable, because filtering with a mask (boolean indexing) does not work anymore. 
So if I want to keep this PR, I will have to distinguish the return type (array vs Index) on the dtype of the result (bool vs numerical/string). Unless we have a bool support in Index.
ehen we have a comparison method that returns a boolean array we just return the array directly 

see  _add_comparison_methods in indexes/base

so i would do the same here, just return the ndarray


@jreback updated this, if you could have a look again

This PR has the consequence that it introduces an inconsistency between the return type of different datetime field accessors (-> array for boolean fields and Index for all others). So we have to be sure we are OK with introducing this.
@jorisvandenbossche yes will put some comments.

FYI don't cancel any travis jobs....testing the deduping auto cancellation.
> This PR has the consequence that it introduces an inconsistency between the return type of different datetime field accessors (-> array for boolean fields and Index for all others). So we have to be sure we are OK with introducing this.

as I said before, I think this is ok. but let me look.
@jorisvandenbossche let's merge this unless anything else (I'll rebase on top after).
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15589?src=pr&el=h1) Report
> Merging [#15589](https://codecov.io/gh/pandas-dev/pandas/pull/15589?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/79581ffe6fb73089dfa8394c2f4e44677acfe1ce?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `100%`.


```diff
@@            Coverage Diff             @@
##           master   #15589      +/-   ##
==========================================
- Coverage   91.02%   90.99%   -0.03%     
==========================================
  Files         143      143              
  Lines       49403    49407       +4     
==========================================
- Hits        44967    44960       -7     
- Misses       4436     4447      +11
```


| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/15589?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/tseries/util.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvdXRpbC5weQ==) | `100% <100%> (ø)` | :arrow_up: |
| [pandas/tseries/common.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29tbW9uLnB5) | `88.09% <100%> (-1.07%)` | :arrow_down: |
| [pandas/tseries/converter.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvY29udmVydGVyLnB5) | `62.95% <100%> (ø)` | :arrow_up: |
| [pandas/tseries/period.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvcGVyaW9kLnB5) | `92.67% <100%> (+0.01%)` | :arrow_up: |
| [pandas/tseries/tdi.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvdGRpLnB5) | `90.23% <100%> (ø)` | :arrow_up: |
| [pandas/tseries/index.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvaW5kZXgucHk=) | `95.4% <100%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/common.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tbW9uLnB5) | `90.96% <0%> (-0.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.86% <0%> (-0.1%)` | :arrow_down: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/15589?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/15589?src=pr&el=footer). Last update [79581ff...ffacd38](https://codecov.io/gh/pandas-dev/pandas/compare/79581ffe6fb73089dfa8394c2f4e44677acfe1ce...ffacd384b04edf5280da74d3bdce2f624a48c24d?el=footer&src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).
thanks!