REF: IntervalIndex[IntervalArray]
State: closed

Closes #19453
Closes #19209


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

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













































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

##### Comment last updated on July 13, 2018 at 01:01 Hours UTC
you copied code from II this is completely he wrong direction to go
instead move it to a shared mixin

>  instead move it to a shared mixin

Could you maybe demo a single method that you think can be moved to a mixin?
this should be an extremely simple wrapper around the Index code 
but if u want to make Index subclass EA that’s hard 

my point is evey method here should be shared
and if not you have to have a really really good reason

> my point is evey method here should be shared
> and if not you have to have a really really good reason

Again, I really don't think there's any code duplication in the current implementation. We've been discussing Indexes subclassing EAs in https://github.com/pandas-dev/pandas/issues/19696, but the prevailing opinion there is that composition is better, specifically https://github.com/pandas-dev/pandas/issues/19696#issuecomment-376284210 and https://github.com/pandas-dev/pandas/issues/19696#issuecomment-377505412. If you want to weigh in there, then please do.

But assuming we do have `IntervalIndex` subclass `IntervalArray`, what does that actually gain? We can eliminate things like

```@property
def left(self):
    return self._data.left
```

Ok... I don't think that's worth the extra time it'll take to rewrite CategoricalIndex and IntervalIndex to use inheritance.
I think also closes #19453, as it achieves the desired end result?  At the very least it looks to implement the behavior I wanted from that issue.  Will try taking a closer look at the changes here after work.
Thanks, forgot about that issue. Adding to the closed list.

On Wed, Apr 4, 2018 at 2:03 PM, jschendel <notifications@github.com> wrote:

> I think also closes #19453
> <https://github.com/pandas-dev/pandas/issues/19453>, as it achieves the
> desired end result? At the very least it looks to implement the behavior I
> wanted from that issue. Will try taking a closer look at the changes here
> after work.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/20611#issuecomment-378710167>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIupYF34AACMJv8OlV5cGnSZ53l8qks5tlRj2gaJpZM4THJIe>
> .
>

# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/20611?src=pr&el=h1) Report
> :exclamation: No coverage uploaded for pull request base (`master@dcbf8b5`). [Click here to learn what that means](https://docs.codecov.io/docs/error-reference#section-missing-base-commit).
> The diff coverage is `93.44%`.

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

```diff
@@            Coverage Diff            @@
##             master   #20611   +/-   ##
=========================================
  Coverage          ?   91.96%           
=========================================
  Files             ?      166           
  Lines             ?    50274           
  Branches          ?        0           
=========================================
  Hits              ?    46232           
  Misses            ?     4042           
  Partials          ?        0
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.35% <93.44%> (?)` | |
| #single | `42.17% <37.11%> (?)` | |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/20611?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/dtypes/missing.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL21pc3NpbmcucHk=) | `93.56% <ø> (ø)` | |
| [pandas/core/dtypes/dtypes.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2R0eXBlcy5weQ==) | `96.04% <100%> (ø)` | |
| [pandas/util/\_doctools.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvX2RvY3Rvb2xzLnB5) | `12.87% <100%> (ø)` | |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `85.68% <100%> (ø)` | |
| [pandas/core/util/hashing.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdXRpbC9oYXNoaW5nLnB5) | `98.36% <100%> (ø)` | |
| [pandas/core/arrays/\_\_init\_\_.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL19faW5pdF9fLnB5) | `100% <100%> (ø)` | |
| [pandas/core/dtypes/concat.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2NvbmNhdC5weQ==) | `99.18% <100%> (ø)` | |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.52% <100%> (ø)` | |
| [pandas/core/arrays/categorical.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2NhdGVnb3JpY2FsLnB5) | `95.94% <100%> (ø)` | |
| [pandas/io/packers.py](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL3BhY2tlcnMucHk=) | `88.04% <85.71%> (ø)` | |
| ... and [3 more](https://codecov.io/gh/pandas-dev/pandas/pull/20611/diff?src=pr&el=tree-more) | |

------

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

Did a basic version of `IntervalArray.__setitem__` in https://github.com/pandas-dev/pandas/pull/20611/commits/de96a61c4d46cc7ca7e4249bd4a0e63bc68b549b.

I think that we have to copy both `left` and `right`, to ensure that we're never in a state where only `left.__setitem__` succeeds, while `right.__setitem__` fails.
(Pulling the `__repr__` discussion out to the main thread.)

So, AFAICT, the only duplicate code between IntervalIndex and IntervalArray is in formatting. This is just like the duplicate code between CategoricalIndex and CategoricalArray. I can move the "array" formatting, which will be common to Index and pandas' extension arrays, to a new mixin class. I can do that as part of this PR, or a followup.

Still need to fixup `__setitem__` calls that modify the type of `left` & `right` and address a few other comments.
Other comment, related to the formatting: for Index we use eg `interval[int64]` as dtype repr to indicate the type of the intervals, but for Series repr it only uses 'interval' (not sure if it is needed there, `Series(..).dtype` still gives `interval[int64]` 
Pushed some pre-rebase commits:
- Added NA support to `__setitem__`
- Implemented `IntervalArray.repeat` to fix some failing tests that were complaining about it not existing
- Addressed a number of review comments
   - Still a few that need to be addressed, but mostly docstring related
Addressed the latest set of review comments, aside from the `_format_data` one (will do in a followup pr or with  another set of review comments).
@jschendel how do you feel about this? IMO, things looked quite nice the last time I looked. Are the outstanding issues

1. Sharing printing between Index and IntervalArray
2. (Maybe) changing `rewrite_exception` to use a method to handle raising with the right name?

Anything else? Are we comfortable doing those as followups? (fine by me)
Yes, I believe those are the only outstanding issues, aside from the question I had about @jreback's last comment.  I'm fine with doing those items as followups.
Cool, +1 from me then.

On Fri, Jul 13, 2018 at 3:10 PM, Jeremy Schendel <notifications@github.com>
wrote:

> Yes, I believe those are the only outstanding issues, aside from the
> question I had about @jreback <https://github.com/jreback>'s last
> comment. I'm fine with doing those items as followups.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/20611#issuecomment-404940685>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIl209JtiUWQNdHlT5hf3MW1J9F5Kks5uGP6ggaJpZM4THJIe>
> .
>

@jschendel what comment? i am ok with merging and doing follow ups as well
@jreback : https://github.com/pandas-dev/pandas/pull/20611#discussion_r201291862

"I suppose should move some of these tests to `pandas/tests/arrays/interval`, maybe leave the extension API specific ones here"

Regardless, doesn't seem like something that would block this from being merged, and could be handled in a followup too.
A run of `asv find` has identified this PR as resulting in a ~1000x slowdown in the `indexing.IntervalIndexing.time_loc_list` benchmark:
```
asv find 7e6e7^..HEAD indexing.IntervalIndexing.time_loc_list
[...]
·· Installing 365eac4d <master~642> into conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
··· Setting up indexing.py:220                                                                                                                                                                         ok
··· Running (indexing.IntervalIndexing.time_loc_list--).
··· indexing.IntervalIndexing.time_loc_list                                                                                                                                                      2.27±0ms
· Testing -----------------------------------O>-----------------------
·· For pandas commit f3160bfa <master~639>:
·· Uninstalling from conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
·· Building f3160bfa <master~639> for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt............................................................................................................
·· Installing f3160bfa <master~639> into conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
··· Setting up indexing.py:220                                                                                                                                                                         ok
··· Running (indexing.IntervalIndexing.time_loc_list--).
··· indexing.IntervalIndexing.time_loc_list                                                                                                                                                    2.90±0.03s
· Testing -----------------------------------O------------------------
·· For pandas commit 610a19a3 <master~641>:
·· Uninstalling from conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
·· Building 610a19a3 <master~641> for conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt.................................................................................................................
·· Installing 610a19a3 <master~641> into conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt..
·· Benchmarking conda-py3.6-Cython-matplotlib-numexpr-numpy-openpyxl-pytables-pytest-scipy-sqlalchemy-xlrd-xlsxwriter-xlwt
··· Setting up indexing.py:220                                                                                                                                                                         ok
··· Running (indexing.IntervalIndexing.time_loc_list--).
··· indexing.IntervalIndexing.time_loc_list                                                                                                                                                    2.92±0.01s.
· Greatest regression found: 610a19a3 <master~641>
```
Bisection done on Arch Linux and reproducible on Ubuntu 18.04.