Unify Index._dir_* with Series implementation
State: closed

Have `Index` use the same implementation of `_dir_additions` and `_dir_deletions` that `Series` currently uses. Move that implementation into a shared base class. @jreback suggested that shared base class `DirNamesMixin` be moved from `core.base` to `core.accessor`.


De-duplicate `_dir_additions` and `_dir_deletions` code.  `Index` currently gets its implementations of these two methods from `strings.StringMethodMixin`, but the implementation is just a special case of the implementation in `Series`.  That implementation is essentially The General Case, so this PR moves it up to a base class `DirNamesMixin`.  (I'm open to putting this elsewhere in the hierarchy)


This PR removes `StringMethodMixin`, discussed in #17042.


For `DatetimeIndex`, `TimeDeltaIndex`, and `PeriodIndex`, this adds a property `dt` that just returns `self`.  This way a user can access e.g. `item.year` symmetrically without having to check whether `item` is a `Series/Index`.

Holding off on writing tests until a consensus is reached.


 - [ ] closes #xxxx
 - [ ] tests added / passed
 - [x] passes ``git diff upstream/master -u -- "*.py" | flake8 --diff``
 - [ ] whatsnew entry


Labels:
name: Internals
description: Related to non-user accessible pandas implementation

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=h1) Report
> Merging [#17117](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/b03f7e52e859c5d20141a47aa4d6880a321af84d?src=pr&el=desc) will **decrease** coverage by `0.02%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #17117      +/-   ##
==========================================
- Coverage   91.02%      91%   -0.03%     
==========================================
  Files         161      161              
  Lines       49397    49391       -6     
==========================================
- Hits        44966    44950      -16     
- Misses       4431     4441      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.78% <100%> (-0.01%)` | :arrow_down: |
| #single | `40.28% <50%> (-0.06%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `96.28% <100%> (+0.07%)` | :arrow_up: |
| [pandas/core/strings.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==) | `98.45% <100%> (-0.03%)` | :arrow_down: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `96.68% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `92.02% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.9% <100%> (-0.05%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `95.94% <100%> (ø)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/io/json/table\_schema.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2lvL2pzb24vdGFibGVfc2NoZW1hLnB5) | `94.36% <0%> (-1.41%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.66% <0%> (-0.1%)` | :arrow_down: |

------

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

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

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

```diff
@@            Coverage Diff             @@
##           master   #17117      +/-   ##
==========================================
+ Coverage      91%   91.01%   +<.01%     
==========================================
  Files         162      163       +1     
  Lines       49508    49561      +53     
==========================================
+ Hits        45055    45108      +53     
  Misses       4453     4453
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `88.79% <100%> (+0.03%)` | :arrow_up: |
| #single | `40.25% <50%> (-0.08%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `95.95% <100%> (-0.06%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `92.04% <100%> (ø)` | :arrow_up: |
| [pandas/core/series.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc2VyaWVzLnB5) | `94.95% <100%> (-0.05%)` | :arrow_down: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `95.93% <100%> (ø)` | :arrow_up: |
| [pandas/core/accessor.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYWNjZXNzb3IucHk=) | `100% <100%> (ø)` | |
| [pandas/core/strings.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvc3RyaW5ncy5weQ==) | `98.45% <100%> (-0.03%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/computation/scope.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvY29tcHV0YXRpb24vc2NvcGUucHk=) | `93.13% <0%> (-0.93%)` | :arrow_down: |
| [pandas/core/nanops.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvbmFub3BzLnB5) | `97.65% <0%> (-0.39%)` | :arrow_down: |
| [pandas/tseries/frequencies.py](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvZnJlcXVlbmNpZXMucHk=) | `96.51% <0%> (-0.16%)` | :arrow_down: |
| ... and [44 more](https://codecov.io/gh/pandas-dev/pandas/pull/17117?src=pr&el=tree-more) | |

------

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

ok your re-factor to string out the accessors is ok. but pls put in a new ``core/accessor.py`` which can be imported into base (IOW it has no other deps).

also remove the .dt/.cat/.str accessors from the Index objects. pls make an issue about this instead; I think its ok, but would like some discussion.

In general try to do 1 change per PR to avoid intermixing things.

I think existing testing should be sufficient for the accessor re-factor (but ok to add if something is not covered).

ping on green.
> ok your re-factor to string out the accessors is ok. but pls put in a new core/accessor.py which can be imported into base (IOW it has no other deps).

I don't understand this sentence.  What is it that you want to keep and what is it that you want in core/accessor.py?  There is a small amount of overlap between this and #17042, but I tried to keep that to a minimum.

> also remove the .dt/.cat/.str accessors from the Index objects. pls make an issue about this instead; I think its ok, but would like some discussion.

OK
> ok your re-factor to string out the accessors is ok. but pls put in a new core/accessor.py which can be imported into base (IOW it has no other deps).

- move the changes that you made in ``core/base.py`` to a new ``core/accessor.py``.
- remove the ``.dt`` etc from ``Index`` for now. (the changes you made to Series are fine).

Hello @jbrockmendel! Thanks for updating the PR.













Cheers ! There are no PEP8 issues in this Pull Request. :beers: 

##### Comment last updated on August 20, 2017 at 15:38 Hours UTC
closing, this is not the impl we want and is confusing.
sorry this one was changed to be reasonable. still need to review.
@jbrockmendel I wanted to review this, but before doing that, some questions/requests:

* can you give some updated explanation of what the PR is now doing? As I thought the scope has changed a bit during its lifetime (or not?) (if appropriate, you can also update the top post, possibly by leaving the original, but adding an updated description at the top)
* Can you give the PR a more descriptive title?
* Is this only internal refactorings in the end? As I don't see a whatsnew note or changes to tests (but if this actually changes behaviour, that should be tested/noted)
@jorisvandenbossche 

> can you give some updated explanation of what the PR is now doing?

You're absolutely right that this has meandered over its lifetime.  It's now pretty limited in scope.

TLDR: Have `Index` use the same implementation of `_dir_additions` and `_dir_deletions` that `Series` currently uses.  Move that implementation into a shared base class.  @jreback suggested that shared base class `DirNamesMixin` be moved from `core.base` to `core.accessor`.

This lets us get rid of `core.strings.StringAccessorMixin` which is currently used about 1.33 times.

> if appropriate, you can also update the top post

Will do.

> Can you give the PR a more descriptive title?

Will do.

> Is this only internal refactorings in the end?

Very nearly.  It is technically changing the behavior of `Index._dir_additions` and `_dir_deletions`, adding an `_accessors` attribute (to match `Series`).  In conjunction with #17134 (pending your signing off on that I'll make a PR) it will handle the `dt` attribute correctly.


>  @jreback suggested that shared base class DirNamesMixin be moved from core.base to core.accessor.

I still feel this seems more appropriate to put in base.py, as it is about adding `__dir__` to the base objects, and is only used in base.py
> I still feel this seems more appropriate to put in base.py, as it is about adding __dir__ to the base objects, and is only used in base.py

This is just as clean and removes some boilerplate from there. So happy where it is (going)
If this is good to go, I can rebase #17042 and tie that off, along with #17134.
thanks!