Recognize timezoned labels when accessing dataframes.
State: closed

- [X] closes #16785 
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Labels:
name: Datetime
description: Datetime data dtype
name: Indexing
description: Related to indexing on series/frames, not to indexes themselves
name: Timezones
description: Timezone data dtype

Comments:
Ouch, looks like some for me...
pls rebase when you can
Hello @1kastner! Thanks for updating the PR.







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

##### Comment last updated on November 26, 2017 at 22:11 Hours UTC
I did a rebase and some more minor adjustments.
Depends on #18095 / #18037 
@jreback To be frank I am rather new to this kind if github set-up and with the rebase now it looks like my pull request is a bit blown up. Is that usual or were some of my git commands not in line with what this setup is optimized for?
@jreback How should the whatsnew entry look like? What I added is that if the datetime index has timezone set and the label used for indexing has a timezone provided then that timezone is considered. The question at hand now is: Is that sufficient?

My idea: if the label contains a timezone but the datetime index does not support that, there should be a user warning. An error seems harsh because maybe the user knows what s/he is doing. Do you think that is a good idea? Or should we just silently ignore the timezone as it was done before?
@1kastner you can

``git rebase -i upstream/master`` to rebase

alternatively you can
``git merge upstream/master`` will pretty much do the same thing
> @jreback How should the whatsnew entry look like? What I added is that if the datetime index has timezone set and the label used for indexing has a timezone provided then that timezone is considered. The question at hand now is: Is that sufficient?

> My idea: if the label contains a timezone but the datetime index does not support that, there should be a user warning. An error seems harsh because maybe the user knows what s/he is doing. Do you think that is a good idea? Or should we just silently ignore the timezone as it was done before?

If the user provides a timezone, then that should be used for indexing. If the target *has* a timezone (including UTC), then this is an unambiguous operation.

If the target timezone is naive, we *could* treat it as UTC for indexing purposes. I think this would be ok (though a warning in this case would be fine).
@jreback My idea was that if no timezone is provided, the timezone of the datetime index is chosen. That seems more intuitive to me. Now we have a matrix:

- datetime index naive, label naive -> treat as in same timezone
- datetime index naive, label with timezone -> ambiguous, user warning, assume UTC?
- datetime index with timezone, label naive -> treat like label has same timezone like index
- datetime index with timezone, label with timezone -> translate label to new timezone

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

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

```diff
@@            Coverage Diff             @@
##           master   #17920      +/-   ##
==========================================
- Coverage   91.25%   91.23%   -0.02%     
==========================================
  Files         163      163              
  Lines       50124    50133       +9     
==========================================
- Hits        45742    45741       -1     
- Misses       4382     4392      +10
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.05% <94.11%> (-0.01%)` | :arrow_down: |
| #single | `40.31% <0%> (-0.07%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.45% <94.11%> (-0.06%)` | :arrow_down: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `25% <0%> (-58.34%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.75% <0%> (-0.1%)` | :arrow_down: |

------

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

```diff
@@            Coverage Diff             @@
##           master   #17920      +/-   ##
==========================================
+ Coverage   91.33%   91.34%   +<.01%     
==========================================
  Files         163      163              
  Lines       49801    49704      -97     
==========================================
- Hits        45487    45401      -86     
+ Misses       4314     4303      -11
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `89.14% <100%> (ø)` | :arrow_up: |
| #single | `39.66% <0%> (-1.11%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/indexes/datetimes.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZXMucHk=) | `95.53% <100%> (-0.19%)` | :arrow_down: |
| [pandas/io/clipboard/clipboards.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2lvL2NsaXBib2FyZC9jbGlwYm9hcmRzLnB5) | `24.05% <0%> (-8.14%)` | :arrow_down: |
| [pandas/core/indexes/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9pbnRlcnZhbC5weQ==) | `92.52% <0%> (-0.7%)` | :arrow_down: |
| [pandas/core/dtypes/cast.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZHR5cGVzL2Nhc3QucHk=) | `88.41% <0%> (-0.11%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `95.73% <0%> (-0.06%)` | :arrow_down: |
| [pandas/core/indexes/multi.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9tdWx0aS5weQ==) | `96.4% <0%> (-0.04%)` | :arrow_down: |
| [pandas/core/indexes/timedeltas.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy90aW1lZGVsdGFzLnB5) | `91.17% <0%> (-0.04%)` | :arrow_down: |
| [pandas/tseries/offsets.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL3RzZXJpZXMvb2Zmc2V0cy5weQ==) | `97% <0%> (-0.03%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `97.8% <0%> (-0.01%)` | :arrow_down: |
| ... and [15 more](https://codecov.io/gh/pandas-dev/pandas/pull/17920?src=pr&el=tree-more) | |

------

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

@jreback for me it looks like ready to go now.
will have a look 
except the timestamp wrapping the other topics have been covered in my last commits.
@jreback do you have any further feedback?
lgtm. can you add a whatsnew note, maybe in Other Enhancements. Also in the partial string indexing doc section, can you add a note (or maybe a warning) about this, and show an example?
ping when you update
@jreback ping on update
The failing test I accidentially got from the master branch.
@jreback pls hold off on this pending resolution of #18435.
@jbrockmendel your discussion w.r.t. to this issue is misplaced. We are talking about how to interpret partial strings when you already know the underlying details about the tz's in question. This is just user convenience and is completely orthogonal to other issues you have raised.
> This is just user convenience and is completely orthogonal to other issues you have raised.

I understand the convenience issue.  Consider the two types of slicing/indexing `series[lower:upper]` and `series[(series.index >= lower) & (series.index <= lower)]`.  This equivalence breaks the orthogonality.  If you want to break the equivalence that's fine, I just want that design decision to be made explicit.
@jreback Is this pull request ready to go?
I am not sure whether that is a good idea. That comment is not mine and it is not related to my code. The proposed refactoring is beyond the scope of this pull request.

*EDIT*: I published this via my email client and as @jbrockmendel pointed out my comment was meant to be for your change request.
> I am not sure whether that is a good idea. That comment is not mine and it is not related to my code. The proposed refactoring is beyond the scope of this pull request.

@1kastner not sure what you are referring. I had only 1 more comment (small). @jbrockmendel concerns will be addressed elsewhere.

@jreback I think the comment @1kastner was referring to was the `#no exception` comment you asked to have removed.  Ambiguity in code-comment vs GH comment.

> I would remove the `# no exception` here
if this was the what you were referring

> I would remove the # no exception here

then let's add a comparison for that test (to ensure it returns the correct thing).
and add a comment as to why there is a warning.

as to being 'out of scope', well just it. since this code needed to be touched it also needs updating. just because there is a comment that is non-sensical, doesn't mean we leave technical debt in place.
@jreback This test checks whether no exception is thrown. Actually it is better to completely remove this test because whatever it tests is covered by my new tests as well

> @jreback This test checks whether no exception is thrown. Actually it is better to completely remove this test because whatever it tests is covered by my new tests as well

that would be fine