-
-
Notifications
You must be signed in to change notification settings - Fork 18.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
BUG: Fix inconsistent ordering between left and right in merge #37406
Conversation
@@ -2283,3 +2283,13 @@ def test_merge_join_categorical_multiindex(): | |||
expected = expected.drop(["Cat", "Int"], axis=1) | |||
result = a.join(b, on=["Cat1", "Int1"]) | |||
tm.assert_frame_equal(expected, result) | |||
|
|||
|
|||
@pytest.mark.parametrize("how", ["left", "right"]) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sort=True is ok? (do we test this)? can you locate this test function near similar tests.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sort True was the previous default behavior. Added tests to show behavior. Moved the tests up a bit right below a nosort test
can you merge master and will look |
� Conflicts: � doc/source/whatsnew/v1.2.0.rst
merged master |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
could do this for 1.2 if you can update and merge master
� Conflicts: � doc/source/whatsnew/v1.2.0.rst
Merged master too |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
small comment ping on green
doc/source/whatsnew/v1.2.0.rst
Outdated
@@ -712,6 +712,7 @@ Reshaping | |||
- Fixed regression in :func:`merge` on merging DatetimeIndex with empty DataFrame (:issue:`36895`) | |||
- Bug in :meth:`DataFrame.apply` not setting index of return value when ``func`` return type is ``dict`` (:issue:`37544`) | |||
- Bug in :func:`concat` resulting in a ``ValueError`` when at least one of both inputs had a non-unique index (:issue:`36263`) | |||
- Bug in :meth:`df.merge() <pandas.DataFrame.merge>` returning inconsistent ordering in result for ``how=right`` and ``how=left`` (:issue:`35382`) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you don't need to add the <...>
instead do this like `:meth:`DataFrame.merge`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
also add :meth:`pandas.merge`
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done
thanks @phofl very nice! |
Thx No they are not related. but #22449 can be closed probably, after we improved the Error Reporting here this should definitely fail as said by @TomAugspurger |
black pandas
git diff upstream/master -u -- "*.py" | flake8 --diff
Passed sort through for right join too. Did not see a reason why not.
On a related note: I think outer joins return a inconsistent order too.
instead of
like
left
andright
now. Would have to add asort
keyword to outer join. Should I open an issue for that?