ENH: clearer error msg for unequal categoricals in merge_asof (GH#26136)
State: closed

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


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Error Reporting
description: Incorrect or improved errors from pandas
name: Categorical
description: Categorical Data Type

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/26242?src=pr&el=h1) Report
> Merging [#26242](https://codecov.io/gh/pandas-dev/pandas/pull/26242?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/9feb3ad92cc0397a04b665803a49299ee7aa1037?src=pr&el=desc) will **decrease** coverage by `<.01%`.
> The diff coverage is `100%`.

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

```diff
@@            Coverage Diff             @@
##           master   #26242      +/-   ##
==========================================
- Coverage   91.97%   91.96%   -0.01%     
==========================================
  Files         175      175              
  Lines       52368    52371       +3     
==========================================
  Hits        48164    48164              
- Misses       4204     4207       +3
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.52% <100%> (ø)` | :arrow_up: |
| #single | `40.69% <0%> (-0.16%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26242?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.49% <100%> (+0.02%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.9% <0%> (-0.12%)` | :arrow_down: |
| [pandas/util/testing.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL3V0aWwvdGVzdGluZy5weQ==) | `90.71% <0%> (+0.1%)` | :arrow_up: |

------

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

```diff
@@            Coverage Diff             @@
##           master   #26242      +/-   ##
==========================================
+ Coverage   91.97%   91.97%   +<.01%     
==========================================
  Files         175      175              
  Lines       52368    52389      +21     
==========================================
+ Hits        48164    48184      +20     
- Misses       4204     4205       +1
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.52% <100%> (ø)` | :arrow_up: |
| #single | `40.72% <0%> (-0.13%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/26242?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.49% <100%> (+0.02%)` | :arrow_up: |
| [pandas/io/gbq.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2dicS5weQ==) | `78.94% <0%> (-10.53%)` | :arrow_down: |
| [pandas/io/excel/\_base.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2lvL2V4Y2VsL19iYXNlLnB5) | `92.13% <0%> (-0.72%)` | :arrow_down: |
| [pandas/core/indexes/datetimelike.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvaW5kZXhlcy9kYXRldGltZWxpa2UucHk=) | `98.13% <0%> (-0.4%)` | :arrow_down: |
| [pandas/core/base.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYmFzZS5weQ==) | `97.98% <0%> (-0.22%)` | :arrow_down: |
| [pandas/plotting/\_core.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL3Bsb3R0aW5nL19jb3JlLnB5) | `83.63% <0%> (-0.14%)` | :arrow_down: |
| [pandas/core/frame.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZnJhbWUucHk=) | `96.9% <0%> (-0.12%)` | :arrow_down: |
| [pandas/core/generic.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvZ2VuZXJpYy5weQ==) | `93.54% <0%> (ø)` | :arrow_up: |
| [pandas/core/arrays/period.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL3BlcmlvZC5weQ==) | `98.53% <0%> (ø)` | :arrow_up: |
| [pandas/core/arrays/interval.py](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvYXJyYXlzL2ludGVydmFsLnB5) | `93.06% <0%> (ø)` | :arrow_up: |
| ... and [10 more](https://codecov.io/gh/pandas-dev/pandas/pull/26242/diff?src=pr&el=tree-more) | |

------

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

Good question, actually. Yes, there is. It's `test_basic_categorical` (in the same file). But before I added the new test in this pull request, that was the only test that exercised the code paths of `merge_asof` for categoricals. I feel there should be more of those... That would be another pull request IMHO. though.
Thanks for clarifying. So this change does introduce unordered Categoricals which might be counter to what merge_asof is supposed to offer (the docstring explicitly states the join field must be ordered).

Do you you see any other instances of that method supporting unordered data? Wondering if we need to further require that Categoricals are ordered to use this method
@WillAyd Indeed, the merge keys should be ordered, otherwise `merge_asof()` makes no sense. Currently, the merge fails later (in `_get_join_indexers()`) with a `ValueError`. But fun fact, there is no test currently in test_merge_asof.py. I've added a test that undordered categorical merge keys raises.

I've also added code to raise a MergeError (instead of a ValueError further down the road). But maybe adding this error check in in `_get_merge_keys()` is not the right place, because by then, the `by=` keys have been added to the list of merge keys.

Is there a reliable way to skip over the `by=` keys in this new test? Or should I put this test in another method? Thanks! 
Hmm that's unfortunate. If it makes things too complicated I'm not against looking that in more detail as a follow up if you can open an issue for it
Cool. I've reverted the last change, and added a comment to explain what's happening currently. I've created #26249 for the other cleanups.
thanks @chrish42 nice patch!