BUG: loc returning wrong elements for non-monotonic DatetimeIndex

doc/source/whatsnew/v1.2.0.rst

@@ -613,6 +613,7 @@ Indexing
 - Bug in indexing on a :class:`Series` or :class:`DataFrame` with a :class:`CategoricalIndex` using listlike indexer that contains elements that are in the index's ``categories`` but not in the index itself failing to raise ``KeyError`` (:issue:`37901`)
 - Bug in :meth:`DataFrame.iloc` and :meth:`Series.iloc` aligning objects in ``__setitem__`` (:issue:`22046`)
 - Bug in :meth:`DataFrame.loc` did not raise ``KeyError`` when missing combination was given with ``slice(None)`` for remaining levels (:issue:`19556`)
+- Bug in :meth:`DataFrame.loc` and :meth:`DataFrame.__getitem__` returning wrong elements for non-monotonic :class:`DatetimeIndex` for string slices (:issue:`33146`)
 
 Missing
 ^^^^^^^

pandas/core/indexes/datetimes.py

@@ -775,42 +775,54 @@ def slice_indexer(self, start=None, end=None, step=None, kind=None):
         if isinstance(end, date) and not isinstance(end, datetime):
             end = datetime.combine(end, time(0, 0))
 
-        try:
-            return Index.slice_indexer(self, start, end, step, kind=kind)
-        except KeyError:
-            # For historical reasons DatetimeIndex by default supports
-            # value-based partial (aka string) slices on non-monotonic arrays,
-            # let's try that.
-            if (start is None or isinstance(start, str)) and (
-                end is None or isinstance(end, str)
-            ):
-                mask = np.array(True)
-                deprecation_mask = np.array(True)
-                if start is not None:
-                    start_casted = self._maybe_cast_slice_bound(start, "left", kind)
-                    mask = start_casted <= self
-                    deprecation_mask = start_casted == self
-
-                if end is not None:
-                    end_casted = self._maybe_cast_slice_bound(end, "right", kind)
-                    mask = (self <= end_casted) & mask
-                    deprecation_mask = (end_casted == self) | deprecation_mask
-
-                if not deprecation_mask.any():
-                    warnings.warn(
-                        "Value based partial slicing on non-monotonic DatetimeIndexes "
-                        "with non-existing keys is deprecated and will raise a "
-                        "KeyError in a future Version.",
-                        FutureWarning,
-                        stacklevel=5,
-                    )
-                indexer = mask.nonzero()[0][::step]
-                if len(indexer) == len(self):
-                    return slice(None)
+        # GH#33146 if start and end are combinations of str and None and Index is not
+        # monotonic, we can not use Index.slice_indexer because it does not honor the
+        # actual elements, is only searching for start and end
+        if not (
+            (
+                (start is None and isinstance(end, str))
+                or (end is None and isinstance(start, str))
+                or (isinstance(start, str) and isinstance(end, str))
+            )
+            and not self.is_monotonic_increasing
+        ):
+            try:
+                return Index.slice_indexer(self, start, end, step, kind=kind)
+            except KeyError:
+                # For historical reasons DatetimeIndex by default supports
+                # value-based partial (aka string) slices on non-monotonic arrays,
+                # let's try that.
+                if (start is None or isinstance(start, str)) and (
+                    end is None or isinstance(end, str)
+                ):
+                    pass
                 else:
-                    return indexer
-            else:
-                raise
+                    raise
+        mask = np.array(True)
+        deprecation_mask = np.array(True)
+        if start is not None:
+            start_casted = self._maybe_cast_slice_bound(start, "left", kind)
+            mask = start_casted <= self
+            deprecation_mask = start_casted == self
+
+        if end is not None:
+            end_casted = self._maybe_cast_slice_bound(end, "right", kind)
+            mask = (self <= end_casted) & mask
+            deprecation_mask = (end_casted == self) | deprecation_mask
+
+        if not deprecation_mask.any():
+            warnings.warn(
+                "Value based partial slicing on non-monotonic DatetimeIndexes "
+                "with non-existing keys is deprecated and will raise a "
+                "KeyError in a future Version.",
+                FutureWarning,
+                stacklevel=5,
+            )
+        indexer = mask.nonzero()[0][::step]
+        if len(indexer) == len(self):
+            return slice(None)
+        else:
+            return indexer
 
     # --------------------------------------------------------------------
 

pandas/tests/indexes/datetimes/test_partial_slicing.py

@@ -331,6 +331,38 @@ def test_partial_slice_doesnt_require_monotonicity(self):
         with pytest.raises(KeyError, match=r"Timestamp\('2014-01-10 00:00:00'\)"):
             nonmonotonic.loc[timestamp:]
 
+    @pytest.mark.parametrize("indexer_end", [None, "2020-01-02 23:59:59.999999999"])
+    def test_loc_getitem_partial_slice_non_monotonicity(self, indexer_end):
+        # GH#33146
+        df = DataFrame(
+            {"a": [1] * 5},
+            index=Index(
+                [
+                    Timestamp("2019-12-30"),
+                    Timestamp("2020-01-01"),
+                    Timestamp("2019-12-25"),
+                    Timestamp("2020-01-02 23:59:59.999999999"),
+                    Timestamp("2019-12-19"),
+                ]
+            ),
+        )
+        expected = DataFrame(
+            {"a": [1] * 2},
+            index=Index(
+                [
+                    Timestamp("2020-01-01"),
+                    Timestamp("2020-01-02 23:59:59.999999999"),
+                ]
+            ),
+        )
+        indexer = slice("2020-01-01", indexer_end)
+
+        result = df[indexer]
+        tm.assert_frame_equal(result, expected)
+
+        result = df.loc[indexer]
+        tm.assert_frame_equal(result, expected)
+
     def test_loc_datetime_length_one(self):
         # GH16071
         df = DataFrame(