BUG: CategoricalIndex allowed reindexing duplicate sources, but not duplicate targets: this is the wrong way around

Author: batterseapower

doc/source/whatsnew/v1.0.0.rst

@@ -106,7 +106,7 @@ Categorical
 ^^^^^^^^^^^
 
 - Added test to assert the :func:`fillna` raises the correct ValueError message when the value isn't a value from categories (:issue:`13628`)
--
+- For :class:`CategoricalIndex`, `DataFrame.reindex` would fail when the targets contained duplicates, and wouldn't fail if the source contained duplicates (:issue:`28107`)
 -
 
 

pandas/core/indexes/category.py

@@ -551,10 +551,6 @@ def get_value(self, series: AnyArrayLike, key: Any):
         # we might be a positional inexer
         return super().get_value(series, key)
 
-    def _can_reindex(self, indexer):
-        """ always allow reindexing """
-        pass
-
     @Appender(_index_shared_docs["where"])
     def where(self, cond, other=None):
         # TODO: Investigate an alternative implementation with
@@ -579,7 +575,6 @@ def reindex(self, target, method=None, level=None, limit=None, tolerance=None):
             Indices of output values in original index
 
         """
-
         if method is not None:
             raise NotImplementedError(
                 "argument method is not implemented for CategoricalIndex.reindex"
@@ -599,9 +594,6 @@ def reindex(self, target, method=None, level=None, limit=None, tolerance=None):
             indexer = None
             missing = []
         else:
-            if not target.is_unique:
-                raise ValueError("cannot reindex with a non-unique indexer")
-
             indexer, missing = self.get_indexer_non_unique(np.array(target))
 
         if len(self.codes) and indexer is not None:

pandas/tests/indexes/test_category.py

@@ -574,41 +574,56 @@ def test_reindexing(self):
             tm.assert_numpy_array_equal(expected, actual)
 
     def test_reindex_dtype(self):
-        c = CategoricalIndex(["a", "b", "c", "a"])
+        c = CategoricalIndex(["a", "b", "c"])
         res, indexer = c.reindex(["a", "c"])
-        tm.assert_index_equal(res, Index(["a", "a", "c"]), exact=True)
-        tm.assert_numpy_array_equal(indexer, np.array([0, 3, 2], dtype=np.intp))
+        tm.assert_index_equal(res, Index(["a", "c"]), exact=True)
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2], dtype=np.intp))
 
-        c = CategoricalIndex(["a", "b", "c", "a"])
+        c = CategoricalIndex(["a", "b", "c"])
         res, indexer = c.reindex(Categorical(["a", "c"]))
 
-        exp = CategoricalIndex(["a", "a", "c"], categories=["a", "c"])
+        exp = CategoricalIndex(["a", "c"], categories=["a", "c"])
         tm.assert_index_equal(res, exp, exact=True)
-        tm.assert_numpy_array_equal(indexer, np.array([0, 3, 2], dtype=np.intp))
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2], dtype=np.intp))
 
-        c = CategoricalIndex(["a", "b", "c", "a"], categories=["a", "b", "c", "d"])
+        c = CategoricalIndex(["a", "b", "c"], categories=["a", "b", "c", "d"])
         res, indexer = c.reindex(["a", "c"])
-        exp = Index(["a", "a", "c"], dtype="object")
+        exp = Index(["a", "c"], dtype="object")
         tm.assert_index_equal(res, exp, exact=True)
-        tm.assert_numpy_array_equal(indexer, np.array([0, 3, 2], dtype=np.intp))
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2], dtype=np.intp))
 
-        c = CategoricalIndex(["a", "b", "c", "a"], categories=["a", "b", "c", "d"])
+        c = CategoricalIndex(["a", "b", "c"], categories=["a", "b", "c", "d"])
         res, indexer = c.reindex(Categorical(["a", "c"]))
-        exp = CategoricalIndex(["a", "a", "c"], categories=["a", "c"])
+        exp = CategoricalIndex(["a", "c"], categories=["a", "c"])
         tm.assert_index_equal(res, exp, exact=True)
-        tm.assert_numpy_array_equal(indexer, np.array([0, 3, 2], dtype=np.intp))
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2], dtype=np.intp))
 
-    def test_reindex_duplicate_target(self):
+    def test_reindex_duplicate_source(self):
         # See GH23963
         c = CategoricalIndex(["a", "b", "c", "a"], categories=["a", "b", "c", "d"])
-        with pytest.raises(ValueError, match="non-unique indexer"):
-            c.reindex(["a", "a", "c"])
+        with pytest.raises(ValueError, match="duplicate axis"):
+            c._can_reindex(["a", "c"])
 
-        with pytest.raises(ValueError, match="non-unique indexer"):
-            c.reindex(
-                CategoricalIndex(["a", "a", "c"], categories=["a", "b", "c", "d"])
+        with pytest.raises(ValueError, match="duplicate axis"):
+            c._can_reindex(
+                CategoricalIndex(["a", "c"], categories=["a", "b", "c", "d"])
             )
 
+    def test_reindex_duplicate_target(self):
+        # See GH25459
+        c = CategoricalIndex(["a", "b", "c"], categories=["a", "b", "c", "d"])
+        res, indexer = c.reindex(["a", "c", "c"])
+        exp = Index(["a", "c", "c"], dtype="object")
+        tm.assert_index_equal(res, exp, exact=True)
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2, 2], dtype=np.intp))
+
+        res, indexer = c.reindex(
+            CategoricalIndex(["a", "c", "c"], categories=["a", "b", "c", "d"])
+        )
+        exp = CategoricalIndex(["a", "c", "c"], categories=["a", "b", "c", "d"])
+        tm.assert_index_equal(res, exp, exact=True)
+        tm.assert_numpy_array_equal(indexer, np.array([0, 2, 2], dtype=np.intp))
+
     def test_reindex_empty_index(self):
         # See GH16770
         c = CategoricalIndex([])

pandas/tests/indexing/test_categorical.py

@@ -46,6 +46,18 @@ def setup_method(self, method):
                 "B": (Series([1, 1, 2, 1, 3, 2]).astype(CDT([3, 2, 1], ordered=False))),
             }
         ).set_index("B")
+        self.df5 = DataFrame(
+            {
+                "A": np.arange(3, dtype="int64"),
+                "B": Series(list("abc")).astype(CDT(list("cabe"))),
+            }
+        ).set_index("B")
+        self.df6 = DataFrame(
+            {
+                "A": np.arange(3, dtype="int64"),
+                "B": (Series([1, 3, 2]).astype(CDT([3, 2, 1], ordered=False))),
+            }
+        ).set_index("B")
 
     def test_loc_scalar(self):
         result = self.df.loc["a"]
@@ -564,89 +576,82 @@ def test_reindexing(self):
 
         # reindexing
         # convert to a regular index
-        result = self.df2.reindex(["a", "b", "e"])
-        expected = DataFrame(
-            {"A": [0, 1, 5, 2, 3, np.nan], "B": Series(list("aaabbe"))}
-        ).set_index("B")
+        result = self.df5.reindex(["a", "b", "e"])
+        expected = DataFrame({"A": [0, 1, np.nan], "B": Series(list("abe"))}).set_index(
+            "B"
+        )
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["a", "b"])
-        expected = DataFrame(
-            {"A": [0, 1, 5, 2, 3], "B": Series(list("aaabb"))}
-        ).set_index("B")
+        result = self.df5.reindex(["a", "b"])
+        expected = DataFrame({"A": [0, 1], "B": Series(list("ab"))}).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["e"])
+        result = self.df5.reindex(["e"])
         expected = DataFrame({"A": [np.nan], "B": Series(["e"])}).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["d"])
+        result = self.df5.reindex(["d"])
         expected = DataFrame({"A": [np.nan], "B": Series(["d"])}).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
         # since we are actually reindexing with a Categorical
         # then return a Categorical
         cats = list("cabe")
 
-        result = self.df2.reindex(Categorical(["a", "d"], categories=cats))
+        result = self.df5.reindex(Categorical(["a", "e"], categories=cats))
         expected = DataFrame(
-            {"A": [0, 1, 5, np.nan], "B": Series(list("aaad")).astype(CDT(cats))}
+            {"A": [0, np.nan], "B": Series(list("ae")).astype(CDT(cats))}
         ).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(Categorical(["a"], categories=cats))
+        result = self.df5.reindex(Categorical(["a"], categories=cats))
         expected = DataFrame(
-            {"A": [0, 1, 5], "B": Series(list("aaa")).astype(CDT(cats))}
+            {"A": [0], "B": Series(list("a")).astype(CDT(cats))}
         ).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["a", "b", "e"])
-        expected = DataFrame(
-            {"A": [0, 1, 5, 2, 3, np.nan], "B": Series(list("aaabbe"))}
-        ).set_index("B")
+        result = self.df5.reindex(["a", "b", "e"])
+        expected = DataFrame({"A": [0, 1, np.nan], "B": Series(list("abe"))}).set_index(
+            "B"
+        )
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["a", "b"])
-        expected = DataFrame(
-            {"A": [0, 1, 5, 2, 3], "B": Series(list("aaabb"))}
-        ).set_index("B")
+        result = self.df5.reindex(["a", "b"])
+        expected = DataFrame({"A": [0, 1], "B": Series(list("ab"))}).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(["e"])
+        result = self.df5.reindex(["e"])
         expected = DataFrame({"A": [np.nan], "B": Series(["e"])}).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
         # give back the type of categorical that we received
-        result = self.df2.reindex(
-            Categorical(["a", "d"], categories=cats, ordered=True)
+        result = self.df5.reindex(
+            Categorical(["a", "e"], categories=cats, ordered=True)
         )
         expected = DataFrame(
-            {
-                "A": [0, 1, 5, np.nan],
-                "B": Series(list("aaad")).astype(CDT(cats, ordered=True)),
-            }
+            {"A": [0, np.nan], "B": Series(list("ae")).astype(CDT(cats, ordered=True))}
         ).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
-        result = self.df2.reindex(Categorical(["a", "d"], categories=["a", "d"]))
+        result = self.df5.reindex(Categorical(["a", "d"], categories=["a", "d"]))
         expected = DataFrame(
-            {"A": [0, 1, 5, np.nan], "B": Series(list("aaad")).astype(CDT(["a", "d"]))}
+            {"A": [0, np.nan], "B": Series(list("ad")).astype(CDT(["a", "d"]))}
         ).set_index("B")
         assert_frame_equal(result, expected, check_index_type=True)
 
         # passed duplicate indexers are not allowed
-        msg = "cannot reindex with a non-unique indexer"
+        msg = "cannot reindex from a duplicate axis"
         with pytest.raises(ValueError, match=msg):
-            self.df2.reindex(["a", "a"])
+            self.df2.reindex(["a", "b"])
 
         # args NotImplemented ATM
         msg = r"argument {} is not implemented for CategoricalIndex\.reindex"
         with pytest.raises(NotImplementedError, match=msg.format("method")):
-            self.df2.reindex(["a"], method="ffill")
+            self.df5.reindex(["a"], method="ffill")
         with pytest.raises(NotImplementedError, match=msg.format("level")):
-            self.df2.reindex(["a"], level=1)
+            self.df5.reindex(["a"], level=1)
         with pytest.raises(NotImplementedError, match=msg.format("limit")):
-            self.df2.reindex(["a"], limit=2)
+            self.df5.reindex(["a"], limit=2)
 
     def test_loc_slice(self):
         # slicing