Repository: pandas-dev/pandas
Pull request: ENH: clearer error msg for unequal categoricals in merge_asof (GH#26136)
- [x] closes #26136 
- [x] tests added / passed
- [x] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [x] whatsnew entry


Problem statement:
Confusing "MergeError: incompatible merge keys [1] category and category, must be the same type"
I'm using `pd.merges_asof()` to merge two dataframes together. The `by` keys are categoricals, but not equal, apparently (one has a few values more than the other). It took me a little bit to figure that out, however, because the error message for this was a bit confusing: "incompatible merge keys [1] category and category, must be the same type". It'd be nice if it was clearer. I can do the pull request, once we figure out how to make this better.

I see the following possible solutions:
1. Special-case the error message for dtypes that take parameters and so are not necessarily all equal. Something a bit like: "incompatible merge keys: both are category, but not equal ones" (Easiest solution.)
2. Make a nicer error message for categories, by digging a little bit into what makes them not equal. Something like "incompatible merge keys: both categories, but the left one has 3 levels more", or "but they have different levels: ..."
3. Change the `__str__` method for CategoricalDType to print something a bit more informative than "category". (No guarantee though that what we would print would always allow people to distinguish two not-equal categoricals as not equal.. unless we were to print out all the levels.)

Anything that sounds good here?

Hints from issue discussion:
This is pretty tricky. Considering both option 1 and 2 to be a form of special casing I wouldn't be in favor of either of them. Not sure how option 3 would end up looking or if it even makes sense.

What would even be the requirements here? That the categoricals being merged would have to be ordered, monotonic, and that the right categorical would have to be a subset of the left?
This is about categoricals on both sides of the `by` key of `pd.merge_asof()`. My superficial understanding is that they need to be equal, because there's the equivalent of a groupby on those happening somewhere under the hood. But I'll let someone who knows more chime in.
@chrish42 this is correct, categorical must be exactly equal. I suppose the error message could be enhanced if you wanted to do a pull request. soln 1 is the most reasonable. the others are non-trivial / hard.

Commit message: ENH: clearer error msg for unequal categoricals in merge_asof (GH#26136)

Patch to review:
diff --git a/doc/source/whatsnew/v0.25.0.rst b/doc/source/whatsnew/v0.25.0.rst
index b2a379d9fe6f5..a2d78ed5db913 100644
--- a/doc/source/whatsnew/v0.25.0.rst
+++ b/doc/source/whatsnew/v0.25.0.rst
@@ -37,6 +37,7 @@ Other Enhancements
 - :class:`RangeIndex` has gained :attr:`~RangeIndex.start`, :attr:`~RangeIndex.stop`, and :attr:`~RangeIndex.step` attributes (:issue:`25710`)
 - :class:`datetime.timezone` objects are now supported as arguments to timezone methods and constructors (:issue:`25065`)
 - :meth:`DataFrame.query` and :meth:`DataFrame.eval` now supports quoting column names with backticks to refer to names with spaces (:issue:`6508`)
+- :func:`merge_asof` now gives a clearer error message when merge keys are categoricals that are not equal (:issue:`26136`)
 
 .. _whatsnew_0250.api_breaking:
 
diff --git a/pandas/core/reshape/merge.py b/pandas/core/reshape/merge.py
index 0837186e33267..bb13b959ebfd2 100644
--- a/pandas/core/reshape/merge.py
+++ b/pandas/core/reshape/merge.py
@@ -1446,10 +1446,18 @@ def _get_merge_keys(self):
         # validate index types are the same
         for i, (lk, rk) in enumerate(zip(left_join_keys, right_join_keys)):
             if not is_dtype_equal(lk.dtype, rk.dtype):
-                raise MergeError("incompatible merge keys [{i}] {lkdtype} and "
-                                 "{rkdtype}, must be the same type"
-                                 .format(i=i, lkdtype=lk.dtype,
-                                         rkdtype=rk.dtype))
+                if (is_categorical_dtype(lk.dtype) and
+                        is_categorical_dtype(rk.dtype)):
+                    # The generic error message is confusing for categoricals.
+                    msg = ("incompatible merge keys [{i}] both sides "
+                           "category, but not equal ones"
+                           .format(i=i))
+                else:
+                    msg = ("incompatible merge keys [{i}] {lkdtype} and "
+                           "{rkdtype}, must be the same type"
+                           .format(i=i, lkdtype=lk.dtype,
+                                   rkdtype=rk.dtype))
+                raise MergeError(msg)
 
         # validate tolerance; must be a Timedelta if we have a DTI
         if self.tolerance is not None:
diff --git a/pandas/tests/reshape/merge/test_merge_asof.py b/pandas/tests/reshape/merge/test_merge_asof.py
index 990892f3ccda3..7eb5f797751eb 100644
--- a/pandas/tests/reshape/merge/test_merge_asof.py
+++ b/pandas/tests/reshape/merge/test_merge_asof.py
@@ -1006,6 +1006,18 @@ def test_merge_datatype_error(self):
         with pytest.raises(MergeError, match=msg):
             merge_asof(left, right, on='a')
 
+    def test_merge_datatype_categorical_error(self):
+        """ Tests merge datatype mismatch error """
+        msg = r'merge keys \[0\] both sides category, but not equal ones'
+
+        left = pd.DataFrame({'left_val': [1, 5, 10],
+                             'a': pd.Categorical(['a', 'b', 'c'])})
+        right = pd.DataFrame({'right_val': [1, 2, 3, 6, 7],
+                              'a': pd.Categorical(['a', 'X', 'c', 'X', 'b'])})
+
+        with pytest.raises(MergeError, match=msg):
+            merge_asof(left, right, on='a')
+
     @pytest.mark.parametrize('func', [lambda x: x, lambda x: to_datetime(x)],
                              ids=['numeric', 'datetime'])
     @pytest.mark.parametrize('side', ['left', 'right'])


Code review comments:
diff_hunk: @@ -1006,6 +1006,18 @@ def test_merge_datatype_error(self):
         with pytest.raises(MergeError, match=msg):
             merge_asof(left, right, on='a')
 
+    def test_merge_datatype_categorical_error(self):
+        """ Tests merge datatype mismatch error """
path: pandas/tests/reshape/merge/test_merge_asof.py
text: @user1:
Can remove docstring for test - we don't have these anywhere else

@author:
Likewise. I do that for `test_merge_datatype_error` too?

@author:
There are actually 4 other uses of docstrings in tests in this file. I convert them all to comments?

@user1:
I guess ignore for now but still remove this. Can have a follow up PR to make that module more consistent