Repository: pandas-dev/pandas
Pull request: BUG: Indexing with UTC offset string no longer ignored
- [x] closes #24076
- [x] closes #16785
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry

Additionally, date strings with UTC offsets that are using in indexing are now validated by the following:

- An error is raised if 2 date strings have different UTC offsets
- An error is raised if the date strings have a UTC offset and the index is not timezone aware.

Problem statement:
ERR: validate partial string indexing with tz-aware end-points
#### Code Sample, a copy-pastable example if possible

```python
import pandas as pd
import dateutil.parser

df = pd.DataFrame(index=pd.date_range('2016-01-01T00:00', '2016-12-31T23:59', freq='T'))

df[
    "2016-01-01T00:00-02:00"
    :
    "2016-01-01T02:03"
]  # returns 124 entries

df[
    dateutil.parser.parse("2016-01-01T00:00-02:00")
    :
    dateutil.parser.parse("2016-01-01T02:03")
]  # returns 4 entries

df[
    pd.Timestamp("2016-01-01T00:00-02:00")
    :
    pd.Timestamp("2016-01-01T02:03")
]  # returns 4 entries
```
#### Problem description

The current behavior is that the time zone information is ignored without any warning when providing strings.

#### Expected Output

Either use the time zone information and only return the four desired entries OR warn the user about the fact that the time zone information is not used

#### Output of ``pd.show_versions()``

<details>

INSTALLED VERSIONS
------------------
commit: None
python: 3.6.0.final.0
python-bits: 32
OS: Windows
OS-release: 10
machine: AMD64
processor: Intel64 Family 6 Model 94 Stepping 3, GenuineIntel
byteorder: little
LC_ALL: None
LANG: None
LOCALE: None.None

pandas: 0.20.1
pytest: None
pip: 9.0.1
setuptools: 28.8.0
Cython: None
numpy: 1.12.1
scipy: None
xarray: None
IPython: None
sphinx: None
patsy: None
dateutil: 2.6.0
pytz: 2017.2
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 2.0.2
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: None
s3fs: None
pandas_gbq: None
pandas_datareader: None
</details>


Slicing DatetimeIndex should be timezone aware
#### Code Sample, a copy-pastable example if possible

```python
# Your code here
import numpy as np
import pandas as pd

idx = pd.DatetimeIndex(start='2018-12-02 14:50:00-07:00', end='2018-12-03 03:11:00-07:00', freq='1min')

df = pd.DataFrame(np.random.randn(len(idx), 1), index=idx, columns=['A'])

df['2018-12-02 21:53:34+00:00':'2018-12-02 22:53:54+00:00'].head()
#                                  A
# 2018-12-02 21:54:00-07:00 -0.647961
# 2018-12-02 21:55:00-07:00  0.112683
# 2018-12-02 21:56:00-07:00 -1.221521
# 2018-12-02 21:57:00-07:00  0.052644
# 2018-12-02 21:58:00-07:00  1.799572

```

#### Problem description

When slicing a DatetimeIndex using a timestamp that has a timezone, the timezone is not used as part o the comparison.

#### Expected Output

```python
df[(df.index >= '2018-12-02 21:53:34+00:00') & (df.index < '2018-12-02 22:53:54+00:00')].head()
#                                  A
# 2018-12-02 14:54:00-07:00  0.578494
# 2018-12-02 14:55:00-07:00  0.170471
# 2018-12-02 14:56:00-07:00  1.078600
# 2018-12-02 14:57:00-07:00  0.376983
# 2018-12-02 14:58:00-07:00 -1.295243

```

#### Output of ``pd.show_versions()``

<details>
INSTALLED VERSIONS
------------------
commit: None
python: 3.7.0.final.0
python-bits: 64
OS: Linux
OS-release: 4.17.0-3rodete2-amd64
machine: x86_64
processor: 
byteorder: little
LC_ALL: None
LANG: en_US.UTF-8
LOCALE: en_US.UTF-8

pandas: 0.23.4
pytest: None
pip: 18.1
setuptools: 40.6.2
Cython: None
numpy: 1.15.4
scipy: 1.1.0
pyarrow: None
xarray: None
IPython: None
sphinx: None
patsy: None
dateutil: 2.7.5
pytz: 2018.7
blosc: None
bottleneck: None
tables: None
numexpr: None
feather: None
matplotlib: 3.0.1
openpyxl: None
xlrd: None
xlwt: None
xlsxwriter: None
lxml: None
bs4: None
html5lib: None
sqlalchemy: None
pymysql: None
psycopg2: None
jinja2: None
s3fs: None
fastparquet: None
pandas_gbq: None
pandas_datareader: None

</details>

Hints from issue discussion:
Your end-point is naive (FYI dateutil.parser is not used anywhere in pandas except as a very last resort as it is slow).

```
In [8]: pd.Timestamp("2016-01-01T00:00-02:00")
Out[8]: Timestamp('2016-01-01 00:00:00-0200', tz='pytz.FixedOffset(-120)')

In [9]: pd.Timestamp("2016-01-01T02:03")
Out[9]: Timestamp('2016-01-01 02:03:00')

In [12]: dateutil.parser.parse("2016-01-01T02:03")
Out[12]: datetime.datetime(2016, 1, 1, 2, 3)

In [13]: dateutil.parser.parse("2016-01-01T00:00-02:00")
Out[13]: datetime.datetime(2016, 1, 1, 0, 0, tzinfo=tzoffset(None, -7200))
```

Full working example
```
In [4]: idx = pd.date_range('2016-01-01T00:00', '2016-12-31T23:59', freq='T')

In [5]: df = pd.DataFrame({'A': range(len(idx))}, index=idx)

In [6]: df
Out[6]: 
                          A
2016-01-01 00:00:00       0
2016-01-01 00:01:00       1
2016-01-01 00:02:00       2
2016-01-01 00:03:00       3
2016-01-01 00:04:00       4
2016-01-01 00:05:00       5
...                     ...
2016-12-31 23:54:00  527034
2016-12-31 23:55:00  527035
2016-12-31 23:56:00  527036
2016-12-31 23:57:00  527037
2016-12-31 23:58:00  527038
2016-12-31 23:59:00  527039

[527040 rows x 1 columns]

In [7]: df["2016-01-01T00:00-02:00":"2016-01-02T00:00-02:00"]
Out[7]: 
                        A
2016-01-01 00:00:00     0
2016-01-01 00:01:00     1
2016-01-01 00:02:00     2
2016-01-01 00:03:00     3
2016-01-01 00:04:00     4
2016-01-01 00:05:00     5
...                   ...
2016-01-01 23:55:00  1435
2016-01-01 23:56:00  1436
2016-01-01 23:57:00  1437
2016-01-01 23:58:00  1438
2016-01-01 23:59:00  1439
2016-01-02 00:00:00  1440

[1441 rows x 1 columns]
```

@1kastner if you are proposing that we raise if the partial-string indexing with a tz must match on both end-points (if given), I would agree.

If you would like to submit a PR would be great. Just step thru to see where the conversions happens.
Thank you very much for this explanation! I will try to find a good spot to insert the proposed exception.
@1kastner want to do a PR for this?
After 17th Oct. (probably a few days earlier) I will have some free time and I'd be interested to solve this. If you prefer to have it solved by somebody else before this date, that'd be no deal for me.
@1kastner totally fine.
So the easiest solution seems now to invoke pd.Timestamp("timestamp-string") on the respective endpoint. I do not agree that the timezones should match. Because in some countries they have daylight saving time, so it is completely normal for [several countries](https://en.wikipedia.org/wiki/Daylight_saving_time) to change their timezone twice a year.

Because of that it is so critical (when not working with UTC) that the timezone information is not ignored.
Well, in `datetimes.py` the parsed offset is totally ignored in the method `_parsed_string_to_bounds`, for my case lines 1292-1296. Instead, the assumed timezone defaults to the timezone of the DatetimeIndex. As said before, because of daylight saving time (also called "summer time" in German), this does not necessarily make sense. You should be free to choose whatever timezone you need for your project, changing with different contexts. I can understand that if no timezone is provided, it defaults to the timezone of the DatetimeIndex timezone instead of UTC. But if a timezone is provided, neglecting it here is not nice.
Please unsubscribe me

On Thu, Oct 19, 2017 at 7:16 AM, 1kastner <notifications@github.com> wrote:

> Well, in datetimes.py the parsed offset is totally ignored in the method
> _parsed_string_to_bounds, for my case lines 1292-1296. Instead, the
> assumed timezone defaults to the timezone of the DatetimeIndex. As said
> before, because of daylight saving time (also called "summer time" in
> German), this does not necessarily make sense. You should be free to choose
> whatever timezone you need for your project, changing with different
> contexts. I can understand that if no timezone is provided, it defaults to
> the timezone of the DatetimeIndex timezone instead of UTC. But if a
> timezone is provided, neglecting it here is not nice.
>
> —
> You are receiving this because you are subscribed to this thread.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/issues/16785#issuecomment-337877144>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/AAJfa05xyf0SkgOM_s_EQhD6A4waACXVks5sty9vgaJpZM4OHoT7>
> .
>



-- 
Greg Gurevich
greggurevich@gmail.com
cell: 1-917-504-7105

@bkmrkr Can't you unsubscribe yourself? 
@jreback Now I struggle with Visual Studio... It might take some some time to properly check everything. But the main idea is to introduce the following change:

```
    def _parsed_string_to_bounds(self, reso, parsed):
        # ...
        if parsed.tzinfo is None:
            target_tz = self.tz
        else:
            target_tz = parsed.tzinfo

        if reso == 'year':
            return (Timestamp(datetime(parsed.year, 1, 1), tz=target_tz),
                    Timestamp(datetime(parsed.year, 12, 31, 23,
                                       59, 59, 999999), tz=target_tz))
    #...
```

Of course throughout all resolutions `self.tz` needs to be replaced with `target_tz`.
@jorisvandenbossche a while back there was a couple of threads about slicing conventions with strings and tz-awareness.  I _think_ this is orthogonal, but this merits another pair of eyes.
@jbrockmendel do you remember where this discussion was? From a search, I found possibly related discussion in https://github.com/pandas-dev/pandas/issues/18435.

I would certainly consider this a bug (but I also have the feeling we have had issues about this before)
See #16785 and related: The timezone information is just dropped during the parsing process, see the related pull request.
This looks fixed in master; could use a test.

```
In [3]: pd.__version__
Out[3]: '0.24.0.dev0+1342.g3e0358d86'

In [4]: df[(df.index >= '2018-12-02 21:53:34+00:00') & (df.index < '2018-12-02 22:53:54+00:00')].head()
Out[4]:
                                  A
2018-12-02 14:54:00-07:00 -0.962576
2018-12-02 14:55:00-07:00  0.324211
2018-12-02 14:56:00-07:00 -0.412128
2018-12-02 14:57:00-07:00 -0.631707
2018-12-02 14:58:00-07:00 -0.594750
```
The changes have not affected #16785 - I just hoped we could have been lucky ;-)
Oops, I think I was looking at the incorrect example; this still looks broken:

```
In [7]: df['2018-12-02 21:53:34+00:00':'2018-12-02 22:53:54+00:00'].head()
   ...:
Out[7]:
                                  A
2018-12-02 21:54:00-07:00 -0.659843
2018-12-02 21:55:00-07:00 -0.802792
2018-12-02 21:56:00-07:00 -1.045828
2018-12-02 21:57:00-07:00  0.059644
2018-12-02 21:58:00-07:00 -2.181424
```

Commit message: isort

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index 4032dc20b2e19..577332f1994a9 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -131,7 +131,7 @@ Interval
 Indexing
 ^^^^^^^^
 
--
+- Bug when a date string with a UTC offset would get ignored during indexing. (:issue:`24076`, :issue:`16785`)
 -
 -
 
diff --git a/pandas/core/indexes/base.py b/pandas/core/indexes/base.py
index cf813f4c3030b..1da82172eb956 100644
--- a/pandas/core/indexes/base.py
+++ b/pandas/core/indexes/base.py
@@ -6,9 +6,10 @@
 import numpy as np
 
 from pandas._libs import (
-    Timedelta, algos as libalgos, index as libindex, join as libjoin, lib,
-    tslibs)
+    algos as libalgos, index as libindex, join as libjoin, lib)
 from pandas._libs.lib import is_datetime_array
+from pandas._libs.tslibs import OutOfBoundsDatetime, Timedelta, Timestamp
+from pandas._libs.tslibs.timezones import tz_compare
 import pandas.compat as compat
 from pandas.compat import range, set_function_name, u
 from pandas.compat.numpy import function as nv
@@ -447,7 +448,7 @@ def __new__(cls, data=None, dtype=None, copy=False, name=None,
                             try:
                                 return DatetimeIndex(subarr, copy=copy,
                                                      name=name, **kwargs)
-                            except tslibs.OutOfBoundsDatetime:
+                            except OutOfBoundsDatetime:
                                 pass
 
                     elif inferred.startswith('timedelta'):
@@ -4866,6 +4867,20 @@ def slice_locs(self, start=None, end=None, step=None, kind=None):
             # If it's a reverse slice, temporarily swap bounds.
             start, end = end, start
 
+        # GH 16785: If start and end happen to be date strings with UTC offsets
+        # attempt to parse and check that the offsets are the same
+        if (isinstance(start, compat.string_types) and
+                isinstance(end, compat.string_types)):
+            try:
+                ts_start = Timestamp(start)
+                ts_end = Timestamp(end)
+            except ValueError:
+                pass
+            else:
+                if not tz_compare(ts_start.tz, ts_end.tz):
+                    raise ValueError("Both date strings must have the same "
+                                     "UTC offset")
+
         start_slice = None
         if start is not None:
             start_slice = self.get_slice_bound(start, 'left', kind)
diff --git a/pandas/core/indexes/datetimes.py b/pandas/core/indexes/datetimes.py
index df91c71cfe238..c1e97b3a64052 100644
--- a/pandas/core/indexes/datetimes.py
+++ b/pandas/core/indexes/datetimes.py
@@ -32,9 +32,8 @@
 from pandas.core.ops import get_op_result_name
 import pandas.core.tools.datetimes as tools
 
-from pandas.tseries import offsets
 from pandas.tseries.frequencies import Resolution, to_offset
-from pandas.tseries.offsets import CDay, prefix_mapping
+from pandas.tseries.offsets import CDay, Nano, prefix_mapping
 
 
 def _new_DatetimeIndex(cls, d):
@@ -826,54 +825,53 @@ def _parsed_string_to_bounds(self, reso, parsed):
         lower, upper: pd.Timestamp
 
         """
+        valid_resos = {'year', 'month', 'quarter', 'day', 'hour', 'minute',
+                       'second', 'minute', 'second', 'microsecond'}
+        if reso not in valid_resos:
+            raise KeyError
         if reso == 'year':
-            return (Timestamp(datetime(parsed.year, 1, 1), tz=self.tz),
-                    Timestamp(datetime(parsed.year, 12, 31, 23,
-                                       59, 59, 999999), tz=self.tz))
+            start = Timestamp(parsed.year, 1, 1)
+            end = Timestamp(parsed.year, 12, 31, 23, 59, 59, 999999)
         elif reso == 'month':
             d = ccalendar.get_days_in_month(parsed.year, parsed.month)
-            return (Timestamp(datetime(parsed.year, parsed.month, 1),
-                              tz=self.tz),
-                    Timestamp(datetime(parsed.year, parsed.month, d, 23,
-                                       59, 59, 999999), tz=self.tz))
+            start = Timestamp(parsed.year, parsed.month, 1)
+            end = Timestamp(parsed.year, parsed.month, d, 23, 59, 59, 999999)
         elif reso == 'quarter':
             qe = (((parsed.month - 1) + 2) % 12) + 1  # two months ahead
             d = ccalendar.get_days_in_month(parsed.year, qe)  # at end of month
-            return (Timestamp(datetime(parsed.year, parsed.month, 1),
-                              tz=self.tz),
-                    Timestamp(datetime(parsed.year, qe, d, 23, 59,
-                                       59, 999999), tz=self.tz))
+            start = Timestamp(parsed.year, parsed.month, 1)
+            end = Timestamp(parsed.year, qe, d, 23, 59, 59, 999999)
         elif reso == 'day':
-            st = datetime(parsed.year, parsed.month, parsed.day)
-            return (Timestamp(st, tz=self.tz),
-                    Timestamp(Timestamp(st + offsets.Day(),
-                                        tz=self.tz).value - 1))
+            start = Timestamp(parsed.year, parsed.month, parsed.day)
+            end = start + timedelta(days=1) - Nano(1)
         elif reso == 'hour':
-            st = datetime(parsed.year, parsed.month, parsed.day,
-                          hour=parsed.hour)
-            return (Timestamp(st, tz=self.tz),
-                    Timestamp(Timestamp(st + offsets.Hour(),
-                                        tz=self.tz).value - 1))
+            start = Timestamp(parsed.year, parsed.month, parsed.day,
+                              parsed.hour)
+            end = start + timedelta(hours=1) - Nano(1)
         elif reso == 'minute':
-            st = datetime(parsed.year, parsed.month, parsed.day,
-                          hour=parsed.hour, minute=parsed.minute)
-            return (Timestamp(st, tz=self.tz),
-                    Timestamp(Timestamp(st + offsets.Minute(),
-                                        tz=self.tz).value - 1))
+            start = Timestamp(parsed.year, parsed.month, parsed.day,
+                              parsed.hour, parsed.minute)
+            end = start + timedelta(minutes=1) - Nano(1)
         elif reso == 'second':
-            st = datetime(parsed.year, parsed.month, parsed.day,
-                          hour=parsed.hour, minute=parsed.minute,
-                          second=parsed.second)
-            return (Timestamp(st, tz=self.tz),
-                    Timestamp(Timestamp(st + offsets.Second(),
-                                        tz=self.tz).value - 1))
+            start = Timestamp(parsed.year, parsed.month, parsed.day,
+                              parsed.hour, parsed.minute, parsed.second)
+            end = start + timedelta(seconds=1) - Nano(1)
         elif reso == 'microsecond':
-            st = datetime(parsed.year, parsed.month, parsed.day,
-                          parsed.hour, parsed.minute, parsed.second,
-                          parsed.microsecond)
-            return (Timestamp(st, tz=self.tz), Timestamp(st, tz=self.tz))
-        else:
-            raise KeyError
+            start = Timestamp(parsed.year, parsed.month, parsed.day,
+                              parsed.hour, parsed.minute, parsed.second,
+                              parsed.microsecond)
+            end = start + timedelta(microseconds=1) - Nano(1)
+        if parsed.tzinfo is not None:
+            if self.tz is None:
+                raise ValueError("The index must be timezone aware "
+                                 "when indexing with a date string with a "
+                                 "UTC offset")
+            start = start.tz_localize(parsed.tzinfo).tz_convert(self.tz)
+            end = end.tz_localize(parsed.tzinfo).tz_convert(self.tz)
+        elif self.tz is not None:
+            start = start.tz_localize(self.tz)
+            end = end.tz_localize(self.tz)
+        return start, end
 
     def _partial_date_slice(self, reso, parsed, use_lhs=True, use_rhs=True):
         is_monotonic = self.is_monotonic
diff --git a/pandas/tests/indexes/datetimes/test_datetime.py b/pandas/tests/indexes/datetimes/test_datetime.py
index e1ba0e1708442..a3ee5fe39769f 100644
--- a/pandas/tests/indexes/datetimes/test_datetime.py
+++ b/pandas/tests/indexes/datetimes/test_datetime.py
@@ -102,7 +102,7 @@ def test_stringified_slice_with_tz(self):
         # GH#2658
         import datetime
         start = datetime.datetime.now()
-        idx = date_range(start=start, freq="1d", periods=10)
+        idx = date_range(start=start, freq="1d", periods=10, tz='US/Eastern')
         df = DataFrame(lrange(10), index=idx)
         df["2013-01-14 23:44:34.437768-05:00":]  # no exception here
 
diff --git a/pandas/tests/indexing/test_datetime.py b/pandas/tests/indexing/test_datetime.py
index 11fb90ebd9bb9..a3d5af78b220a 100644
--- a/pandas/tests/indexing/test_datetime.py
+++ b/pandas/tests/indexing/test_datetime.py
@@ -2,6 +2,7 @@
 
 from dateutil import tz
 import numpy as np
+import pytest
 
 import pandas as pd
 from pandas import DataFrame, Index, Series, Timestamp, date_range
@@ -313,3 +314,20 @@ def test_loc_setitem_with_existing_dst(self):
                                 columns=['value'],
                                 dtype=object)
         tm.assert_frame_equal(result, expected)
+
+    def test_getitem_with_datestring_with_UTC_offset(self):
+        # GH 24076
+        idx = pd.date_range(start='2018-12-02 14:50:00-07:00',
+                            end='2018-12-03 15:00:00-07:00', freq='1min')
+        df = pd.DataFrame(1, index=idx, columns=['A'])
+        result = df['2018-12-02 21:50:00+00:00':'2018-12-02 21:52:00+00:00']
+        expected = df.iloc[0:3, :]
+        tm.assert_frame_equal(result, expected)
+
+        # GH 16785
+        with pytest.raises(ValueError, match="Both date strings"):
+            df['2018-12-02 21:50:00+00:00':'2018-12-02 21:52:00+01:00']
+
+        with pytest.raises(ValueError, match="The index must be timezone"):
+            df = df.tz_localize(None)
+            df['2018-12-02 21:50:00+00:00':'2018-12-02 21:52:00+00:00']


Code review comments:
diff_hunk: @@ -4866,6 +4867,20 @@ def slice_locs(self, start=None, end=None, step=None, kind=None):
             # If it's a reverse slice, temporarily swap bounds.
             start, end = end, start
 
+        # GH 16785: If start and end happen to be date strings with UTC offsets
+        # attempt to parse and check that the offsets are the same
+        if (isinstance(start, compat.string_types) and
+                isinstance(end, compat.string_types)):
+            try:
path: pandas/core/indexes/base.py
text: @user1:
should these not be individually converted?

@author:
They are eventually, but when they are individually converted (after 3 nested function calls), it's impossible to infer what the UTC offset was in the original strings.

This is the earliest opportunity to check that the UTC offsets are the same.

diff_hunk: @@ -4866,6 +4867,20 @@ def slice_locs(self, start=None, end=None, step=None, kind=None):
             # If it's a reverse slice, temporarily swap bounds.
             start, end = end, start
 
+        # GH 16785: If start and end happen to be date strings with UTC offsets
+        # attempt to parse and check that the offsets are the same
+        if (isinstance(start, compat.string_types) and
+                isinstance(end, compat.string_types)):
+            try:
+                ts_start = Timestamp(start)
+                ts_end = Timestamp(end)
+            except ValueError:
+                pass
+            else:
+                if not tz_compare(ts_start.tz, ts_end.tz):
path: pandas/core/indexes/base.py
text: @user1:
shouldn't this happen regardless if these are strings or not?

e.g. df[pd.Timestamp(...):'string']

@author:
Hmm didn't know we supported `df[pd.Timestamp(...):'date_string']`. If that's the case then definitely.