BUG: right merge not preserve row order (#27453)
State: closed

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


Labels:
name: Reshaping
description: Concat, Merge/Join, Stack/Unstack, Explode

Comments:
Not sure how to interpret these errors. Azure is reporting a failure on `test_drop_duplicates_categorical_non_bool`, but that is marked to `xfail` in the [test code](https://github.com/pandas-dev/pandas/blob/master/pandas/tests/series/test_analytics.py#L1492-L1496). Any advice appreciated. @TomAugspurger 
> Azure is reporting a failure on test_drop_duplicates_categorical_non_bool, but that is marked to xfail in the test code. Any advice appreciated.

That xfail references https://github.com/pandas-dev/pandas/issues/7996 if it's any help.

The 3.7 CI failures should be fixed if you merge master & repush.
@WillAyd I cleaned up the sloppy logic [here](https://github.com/pandas-dev/pandas/pull/27762/files#diff-14fc0a514ad9ce2a73eeaa9ec3fb9406L1263). Should be much better now.
The failing tests are unrelated to this PR.
```
=================================== FAILURES ===================================
_ TestCategoricalSeriesAnalytics.test_drop_duplicates_categorical_non_bool[True-datetime64[D]] _
[gw1] linux -- Python 3.7.3 /home/travis/miniconda3/envs/pandas-dev/bin/python
[XPASS(strict)] GH#7996
_ TestCategoricalSeriesAnalytics.test_drop_duplicates_categorical_non_bool[False-datetime64[D]] _
[gw1] linux -- Python 3.7.3 /home/travis/miniconda3/envs/pandas-dev/bin/python
[XPASS(strict)] GH#7996
_ TestCategoricalSeriesAnalytics.test_drop_duplicates_categorical_non_bool[None-datetime64[D]] _
[gw1] linux -- Python 3.7.3 /home/travis/miniconda3/envs/pandas-dev/bin/python
[XPASS(strict)] GH#7996
```

[source](https://travis-ci.org/pandas-dev/pandas/jobs/571136170#L2345)
I addressed all the issues that were brought up. Ready for another round of review. 
Can you resolve merge conflict? Also should only need one whatsnew entry
> Can you resolve merge conflict? Also should only need one whatsnew entry

AS per @jreback 's request, I have a whatsnew entry to both the breaking API changes and the Reshaping sections.
Hello @ncernek! Thanks for updating this PR. We checked the lines you've touched for [PEP 8](https://www.python.org/dev/peps/pep-0008) issues, and found:





There are currently no PEP 8 issues detected in this Pull Request. Cheers! :beers: 

##### Comment last updated at 2020-01-23 03:21:01 UTC
> AS per @jreback 's request, I have a whatsnew entry to both the breaking API changes and the Reshaping sections.

I think the request there was to have this show up in Backwards Incompatible API Changes with its own section, not to duplicate it. You can find examples of how to do that in prior whatsnew notes, like here:

https://github.com/pandas-dev/pandas/blob/master/doc/source/whatsnew/v0.25.0.rst#backwards-incompatible-api-changes
Also looks like a few lint issues need to be addressed
Does anyone understand what the error is here? This should be good to merge.
> This should be good to merge.

Still need to address the comment around the whatsnew. Please take a look at https://github.com/pandas-dev/pandas/pull/27762#issuecomment-541747380

The existing CI failure will probably go away on next push - we've been having intermittent timeouts the past few days that I think should be over now
> Still need to address the comment around the whatsnew. Please take a look at #27762 (comment)

I already took care of this is in [this commit.](https://github.com/pandas-dev/pandas/pull/27762/commits/077b948a9726a2be51b6fe6713efb4f31b976c8e)
@ncernek some of the CI failures look related. e.g. https://dev.azure.com/pandas-dev/pandas/_build/results?buildId=20073
@ncernek any chance you can check CI failures and resolve conflicts?
hey, yeah I can take a look later this week.

On Mon, Dec 9, 2019 at 11:09 William Ayd <notifications@github.com> wrote:

> @ncernek <https://github.com/ncernek> any chance you can check CI
> failures and resolve conflicts?
>
> —
> You are receiving this because you were mentioned.
> Reply to this email directly, view it on GitHub
> <https://github.com/pandas-dev/pandas/pull/27762?email_source=notifications&email_token=ADNDK64BEGE53SA6QDMEXNDQX2CXHA5CNFSM4IJOCUZKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEGKDLAQ#issuecomment-563361154>,
> or unsubscribe
> <https://github.com/notifications/unsubscribe-auth/ADNDK6Z5SI6IHYX4GWLLN7LQX2CXHANCNFSM4IJOCUZA>
> .
>
-- 
-
312.450.5311

@ncernek able to update? We're planning to release the 1.0 rc this week.
Hmm I'm getting an error locally when running tests. Any ideas?
```
╰─$ pytest pandas/tests/reshape/merge/test_merge.py::test_merge_preserves_row_order                                      1 ↵
ImportError while loading conftest '/Users/nico/code/pandas-ncernek/pandas/conftest.py'.
pandas/__init__.py:56: in <module>
    from pandas.core.api import (
pandas/core/api.py:4: in <module>
    from pandas._libs.missing import NA
E   ImportError: cannot import name 'NA' from 'pandas._libs.missing' (/Users/nico/code/pandas-ncernek/pandas/_libs/missing.cpython-37m-darwin.so)
```
> Hmm I'm getting an error locally when running tests. Any ideas?
> 
> ```
> ╰─$ pytest pandas/tests/reshape/merge/test_merge.py::test_merge_preserves_row_order                                      1 ↵
> ImportError while loading conftest '/Users/nico/code/pandas-ncernek/pandas/conftest.py'.
> pandas/__init__.py:56: in <module>
>     from pandas.core.api import (
> pandas/core/api.py:4: in <module>
>     from pandas._libs.missing import NA
> E   ImportError: cannot import name 'NA' from 'pandas._libs.missing' (/Users/nico/code/pandas-ncernek/pandas/_libs/missing.cpython-37m-darwin.so)
> ```

Can you try [rebuilding the C extensions](https://dev.pandas.io/docs/development/contributing.html#creating-a-python-environment)?
I'm not working on this PR anymore. This dev experience is painful. I would have appreciated commits from others to help me get across the finish line a long time ago, esp. when the only issues remaining for the past few months were concerning documentation. 

Separately, uncertain as to why there are hundreds of test failures. These aren't caused by my contribution that I can tell. That doesn't leave me excited to resolve anything anymore.

Anyone else is free to take ownership of this PR. 
> I'm not working on this PR anymore.

That's OK, thanks for letting us know :)

> Separately, uncertain as to why there are hundreds of test failures. These aren't caused by my contribution that I can tell. That doesn't leave me excited to resolve anything anymore.

For a start, it looks like you're calling `assert_frame_equal(expected, result)` (should be `tm.assert_frame_equal(expected, result)`), which is causing a `NameError`

> Anyone else is free to take ownership of this PR.

Sure, will do