BUG-24212 fix regression in #24897
State: closed

- [ ] closes #24897
- [x] tests added / passed
- [X] passes `git diff upstream/master -u -- "*.py" | flake8 --diff`
- [ ] whatsnew entry

Use an extra take to ensure the `other_index` is arranged correctly for replacing values (in the original issue, the `other_index` would've taken from [0, 1, 2, 3] and be left unchanged, so this step was overlooked)

Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode
name: Regression
description: Functionality that used to work in a prior pandas version

Comments:
# [Codecov](https://codecov.io/gh/pandas-dev/pandas/pull/24916?src=pr&el=h1) Report
> Merging [#24916](https://codecov.io/gh/pandas-dev/pandas/pull/24916?src=pr&el=desc) into [master](https://codecov.io/gh/pandas-dev/pandas/commit/5761e359b65631db491349a65fc21d0da51dcc0f?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/24916/graphs/tree.svg?width=650&token=eZ4WkYLtcO&height=150&src=pr)](https://codecov.io/gh/pandas-dev/pandas/pull/24916?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           master   #24916      +/-   ##
==========================================
+ Coverage   92.38%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52404    52415      +11     
==========================================
+ Hits        48413    48424      +11     
  Misses       3991     3991
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <100%> (ø)` | :arrow_up: |
| #single | `42.89% <7.69%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24916?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24916/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.49% <100%> (+0.08%)` | :arrow_up: |

------

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

```diff
@@            Coverage Diff             @@
##           master   #24916      +/-   ##
==========================================
+ Coverage   92.38%   92.38%   +<.01%     
==========================================
  Files         166      166              
  Lines       52404    52414      +10     
==========================================
+ Hits        48413    48423      +10     
  Misses       3991     3991
```

| Flag | Coverage Δ | |
|---|---|---|
| #multiple | `90.8% <100%> (ø)` | :arrow_up: |
| #single | `42.89% <8.33%> (-0.01%)` | :arrow_down: |

| [Impacted Files](https://codecov.io/gh/pandas-dev/pandas/pull/24916?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [pandas/core/reshape/merge.py](https://codecov.io/gh/pandas-dev/pandas/pull/24916/diff?src=pr&el=tree#diff-cGFuZGFzL2NvcmUvcmVzaGFwZS9tZXJnZS5weQ==) | `94.48% <100%> (+0.07%)` | :arrow_up: |

------

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

Feel free to add sections.

On Thu, Jan 24, 2019 at 8:32 PM Justin Zheng <notifications@github.com>
wrote:

> *@JustinZhengBC* commented on this pull request.
> ------------------------------
>
> In doc/source/whatsnew/v0.24.1.rst
> <https://github.com/pandas-dev/pandas/pull/24916#discussion_r250849509>:
>
> > @@ -63,6 +63,9 @@ Bug Fixes
>  -
>  -
>
> +**Reshaping**
> +
> +- Bug in :func:`merge` when merging by index name would sometimes result in an incorrectly numbered index (:issue:`24212`)
>
> There's a reshaping section in the 0.24.0 one. If not under reshaping,
> would it be correct to put this under indexing?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/24916#discussion_r250849509>,
> or mute the thread
> <https://github.com/notifications/unsubscribe-auth/ABQHIm3oaDMcWzWrhdAjGdxCjUp895WMks5vGmzMgaJpZM4aRvs7>
> .
>

thanks @JustinZhengBC 
Given that this fix has given problem before, let's leave this one for 0.25 ? 
maybe thought this was a bug fix
I changed this from 0.24.1 to 0.25.0.

One specific case that seems to be broken by this that I found, is using a categorical as the merge key:

```
In [15]: left = pd.DataFrame({'a': [1, 2, 3], 'key': pd.Categorical(['a', 'a', 'b'], categories=['a', 'b', 'c'])}) 
    ...: right = pd.DataFrame({'b': [1, 2, 3]}, index=pd.Categorical(['a', 'b', 'c']))                                                                                                                              

In [16]: left.merge(right,  left_on='key', right_index=True, how='right')                                                                                                                                           
---------------------------------------------------------------------------
ValueError                                Traceback (most recent call last)
<ipython-input-16-e08f8fc28c75> in <module>
----> 1 left.merge(right,  left_on='key', right_index=True, how='right')

~/scipy/pandas/pandas/core/frame.py in merge(self, right, how, on, left_on, right_on, left_index, right_index, sort, suffixes, copy, indicator, validate)
   6875                      right_on=right_on, left_index=left_index,
   6876                      right_index=right_index, sort=sort, suffixes=suffixes,
-> 6877                      copy=copy, indicator=indicator, validate=validate)
   6878 
   6879     def round(self, decimals=0, *args, **kwargs):

~/scipy/pandas/pandas/core/reshape/merge.py in merge(left, right, how, on, left_on, right_on, left_index, right_index, sort, suffixes, copy, indicator, validate)
     46                          copy=copy, indicator=indicator,
     47                          validate=validate)
---> 48     return op.get_result()
     49 
     50 

~/scipy/pandas/pandas/core/reshape/merge.py in get_result(self)
    544                 self.left, self.right)
    545 
--> 546         join_index, left_indexer, right_indexer = self._get_join_info()
    547 
    548         ldata, rdata = self.left._data, self.right._data

~/scipy/pandas/pandas/core/reshape/merge.py in _get_join_info(self)
    762                     join_index = self.right.index.take(right_indexer)
    763                     left_indexer = np.array([-1] * len(join_index))
--> 764             elif self.left_index:
    765                 if len(self.right) > 0:
    766                     join_index = self.right.index.take(right_indexer)

~/scipy/pandas/pandas/core/reshape/merge.py in _create_join_index(self, index, other_index, indexer, other_indexer, how)
    811 
    812         # ugh, spaghetti re #733
--> 813         if _any(self.left_on) and _any(self.right_on):
    814             for lk, rk in zip(self.left_on, self.right_on):
    815                 if is_lkey(lk):

ValueError: invalid literal for int() with base 10: 'c'
```

The above is failing on master now, but works on 0.23.4 / 0.24.0.
@JustinZhengBC can you have a look at my comments above?
@jorisvandenbossche sorry for not responding earlier. I looked into it and I believe the main issue is not caused by the fact that the breaking example uses a categorical, but that the other index uses a different dtype, so there is no trivial way to choose how to take values (`other_index` adds a new value for the column being joined on, but there's no corresponding value for the index). I made a new PR where the new value is assumed to be the row number of the missing value in `other_index` linked above, but this has the disadvantage that if the original index has a non-numerical dtype that is different from the dtype of the value being joined on, then the index of the resulting `DataFrame` will have mixed dtypes, so I'd appreciate any other suggestions.
@JustinZhengBC no problem! and thanks for looking into it

For my understanding, can you try to explain why the new `_create_join_index` was needed originally?


Sure. Suppose we were merging these two `DataFrame`s using the 'key' column in the first and the index of the second:
```
>>> df1
   a  key
0  1    0
1  2    2
2  3    2
>>> df2
   b
0  1
1  2
2  3
3  4
4  5
```
Originally, `Index.take` would compute a `left_indexer` of [0, 1, 2, -1, -1, -1] to indicate that the resulting frame will have values from `df1` in order for the first three rows, but the rest of the rows will have missing values in columns from `df1`. The problem is that to compute the index, it used to call `self.left.take(left_indexer)` which would result in the positions with -1 taking on the value of the last element in `df1['key']`, resulting in an incorrect index of [0, 1, 2, 2, 2, 2]. The intended result is that the rows that were not in `df1` should be indexed according to their index in `df2`. In this example, that would be [0, 1, 2, 1, 3, 4]. Since we had to pull multiple different values from `df2` to fill in the -1's, we could not use the `fill_value` parameter in `Index.take`.