REF: Pieces broken off of #24024
State: closed

cc @TomAugspurger 

If I did this right, this shouldn't cause rebasing headaches.  LMK if I messed up on that and I'll try to fix.

Nearly everything here is verbatim from #24024.  I'll comment inline with pieces that are not.

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

Comments:
In terms of LOC this shaves about 5% off of 24024.  Not huge, but along with searchsorted, repeat, and reductions getting broken off, these add up
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=h1) Report
> Merging [#24364](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f6cf7d9e2606e82fd3934e1dac8119c43f5ee643?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/24364/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24364      +/-   ##
==========================================
+ Coverage   92.29%   92.29%   +<.01%     
==========================================
  Files         162      162              
  Lines       51832    51834       +2     
==========================================
+ Hits        47839    47841       +2     
  Misses       3993     3993
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.7% <100%> (ø)` | :arrow_up: |
| #single | `42.99% <59.52%> (+0.01%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `96.46% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `97.3% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `87.38% <100%> (+0.22%)` | :arrow_up: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `98.24% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `90.39% <100%> (-0.03%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.5% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.3% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.32% <100%> (-0.01%)` | :arrow_down: |
| [pandas/core/tools/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdG9vbHMvZGF0ZXRpbWVzLnB5) | `85.32% <100%> (+0.04%)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.27% <100%> (ø)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree-more) | |

------

[Continue to review full report at Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24364?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/24364?src=pr&el=footer). Last update [f6cf7d9...d800f2b](https://codecov.io/gh/pandas-dev/pandas/pull/24364?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/24364?src=pr&el=h1) Report
> Merging [#24364](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/f6cf7d9e2606e82fd3934e1dac8119c43f5ee643?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/24364/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24364      +/-   ##
==========================================
+ Coverage   92.29%   92.29%   +<.01%     
==========================================
  Files         162      162              
  Lines       51832    51834       +2     
==========================================
+ Hits        47839    47841       +2     
  Misses       3993     3993
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.7% <100%> (ø)` | :arrow_up: |
| #single | `42.99% <59.52%> (+0.01%)` | :arrow_up: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24364?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/arrays/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lbGlrZS5weQ==) | `96.46% <100%> (+0.01%)` | :arrow_up: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `97.3% <100%> (ø)` | :arrow_up: |
| [pandas/core/arrays/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3RpbWVkZWx0YXMucHk=) | `87.38% <100%> (+0.22%)` | :arrow_up: |
| [pandas/core/arrays/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2RhdGV0aW1lcy5weQ==) | `98.24% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `90.39% <100%> (-0.03%)` | :arrow_down: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.5% <100%> (ø)` | :arrow_up: |
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.3% <100%> (ø)` | :arrow_up: |
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `96.32% <100%> (-0.01%)` | :arrow_down: |
| [pandas/core/tools/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvdG9vbHMvZGF0ZXRpbWVzLnB5) | `85.32% <100%> (+0.04%)` | :arrow_up: |
| [pandas/core/indexes/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9iYXNlLnB5) | `96.27% <100%> (ø)` | :arrow_up: |
| ... and [2 more](https://codecov.io/gh/pandas-dev/pandas/pull/24364/diff?src=pr&el=tree-more) | |

------

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

#24024 doesn't apply cleanly on top of this.

Could we instead give a +1 to the changes here, and then you make a PR against my branch for #24024 with the new changes?
> Could we instead give a +1 to the changes here, and then you make a PR against my branch for #24024 with the new changes?

Will do.
What do you mean by globally changing the hasnans? I don't think anyone was
suggesting changing that anywhere other than on the datelike Array classes,
where we (I) don't want them as part of the public API.

On Fri, Dec 21, 2018 at 11:06 AM Jeff Reback <notifications@github.com>
wrote:

> *@jreback* requested changes on this pull request.
>
> i would split out the hasnans change and do that separately as it needs to
> happen globally
>
> (bamboo-dev) jreback@dev:~/pandas-dev$ grep -r hasnans pandas --include '*.py'|cut -d ' ' -f 1|uniq
> pandas/core/arrays/timedeltas.py:
> pandas/core/arrays/period.py:
> pandas/core/arrays/datetimelike.py:
> pandas/core/arrays/datetimes.py:
> pandas/core/indexes/timedeltas.py:
> pandas/core/indexes/interval.py:
> pandas/core/indexes/category.py:
> pandas/core/indexes/numeric.py:
> pandas/core/indexes/datetimelike.py:
> pandas/core/indexes/base.py:
> pandas/core/dtypes/dtypes.py:
> pandas/core/series.py:
> pandas/core/resample.py:
> pandas/core/base.py:
> pandas/tests/test_lib.py:
> pandas/tests/indexes/datetimes/test_ops.py:
> pandas/tests/indexes/interval/test_interval.py:
> pandas/tests/indexes/period/test_ops.py:
> pandas/tests/indexes/multi/test_missing.py:
> pandas/tests/indexes/multi/test_missing.py:def
> pandas/tests/indexes/multi/test_missing.py:
> pandas/tests/indexes/common.py:
> pandas/tests/indexes/timedeltas/test_ops.py:
> pandas/tests/indexes/test_common.py:
> pandas/tests/series/test_internals.py:def
> pandas/tests/series/test_internals.py:
>
> ------------------------------
>
> In pandas/core/arrays/datetimelike.py
> <https://github.com/pandas-dev/pandas/pull/24364#discussion_r243638426>:
>
> > @@ -451,7 +464,7 @@ def _isnan(self):
>          return (self.asi8 == iNaT)
>
>      @property  # NB: override with cache_readonly in immutable subclasses
> -    def hasnans(self):
> +    def _hasnans(self):
>
> IIRC this is a general Index property, are changing this globally? (ok by
> me)
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/24364#pullrequestreview-187469604>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIs3VybVXypznl-NUsnhsqll10BnBks5u7RUFgaJpZM4ZbLQI>
> .
>

> I don't think anyone was suggesting changing that anywhere other than on the datelike Array classes,

This matches what I had in mind, just doing it on DTI/TDI/PI.
You wrote DTI/TDI/PI, did you mean DTA/TDA/PA? Or do you mean aliasing
_hasnans to hasnans on the index classes? I should probably just look at
the diff :)

On Fri, Dec 21, 2018 at 11:19 AM jbrockmendel <notifications@github.com>
wrote:

> I don't think anyone was suggesting changing that anywhere other than on
> the datelike Array classes,
>
> This matches what I had in mind, just doing it on DTI/TDI/PI.
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/24364#issuecomment-449446722>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIluljjVWby0HQEs-9VSqTVrke2Kwks5u7Rg3gaJpZM4ZbLQI>
> .
>

Probably a typo.  Lots of open windows ATM.
> What do you mean by globally changing the hasnans? I don't think anyone was
suggesting changing that anywhere other than on the datelike Array classes,
where we (I) don't want them as part of the public API.

that's fine, but wont' accept a change that also does not fix the existing Index codebases. This is SO confusing you need as much consistency as possible.
I'm pretty confused right now, so I'll restate what Brock and I are saying.

1. The `hasnans` property shouldn't be part of the public API for Datetime-like arrays. It's duplicative with `isna()`
2. Index.hasnans is a well-established API that we don't want to change.

Maybe you could restate what you're looking for @jreback?
Did we ever reach a consensus here?
thanks. I realized you are just changing the non-Index idiom here which is in-line with other idioms.