-
- Notifications
You must be signed in to change notification settings - Fork 19.3k
BUG:Sanity check on merge parameters for correct exception #26824 #26855
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
Changes from all commits
c3064e6 c862ec0 0df85f0 dea59fa 53aee36 6396947 11b1894 7b4aa22 bfb7984 1e2b276 0f88c32 a9905bd a939d8e 83f8cda 962882d 139d696 4e6bd89 a0680e0 0a894a9 0cb8843 4e45c75 500e374 773dec2 7770b1d efb0761 6b7f5a2 b7c482e e4e486c File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -1091,13 +1091,19 @@ def _validate_specification(self): | |
| raise ValueError('len(left_on) must equal the number ' | ||
| 'of levels in the index of "right"') | ||
| self.right_on = [None] * n | ||
| if not self.right_on: | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. can we instead try to set self.left_on = self.left_on or [None] * len( self.right_on) before these if clauses; that way don’t need to add additional checks Author There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Sure | ||
| raise ValueError('both "right_on" and "left_on" ' | ||
| 'should be passed') | ||
| elif self.right_on is not None: | ||
| n = len(self.right_on) | ||
| if self.left_index: | ||
| if len(self.right_on) != self.left.index.nlevels: | ||
| raise ValueError('len(right_on) must equal the number ' | ||
| 'of levels in the index of "left"') | ||
| self.left_on = [None] * n | ||
| if not self.left_on: | ||
| raise ValueError('both "right_on" and "left_on" ' | ||
| 'should be passed') | ||
| if len(self.right_on) != len(self.left_on): | ||
| raise ValueError("len(right_on) must equal len(left_on)") | ||
| | ||
| | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| | @@ -1026,6 +1026,22 @@ def test_validation(self): | |
| result = merge(left, right, on=['a', 'b'], validate='1:1') | ||
| assert_frame_equal(result, expected_multi) | ||
| | ||
| @pytest.mark.parametrize('merge_type', ['left_on', 'right_on']) | ||
| def test_missing_on_raises(self, merge_type): | ||
| # GH26824 | ||
| left = DataFrame({ | ||
| 'A': [1, 2, 3, 4, 5, 6], | ||
| 'B': ['P', 'Q', 'R', 'S', 'T', 'U'] | ||
| }) | ||
| right = DataFrame({ | ||
| 'A': [1, 2, 4, 5, 7, 8], | ||
| 'C': ['L', 'M', 'N', 'O', 'P', 'Q'] | ||
| }) | ||
| msg = 'must equal' | ||
| kwargs = {merge_type: 'A'} | ||
| with pytest.raises(ValueError, match=msg): | ||
| pd.merge(left, right, how='left', **kwargs) | ||
| | ||
| def test_merge_two_empty_df_no_division_error(self): | ||
| # GH17776, PR #17846 | ||
| a = pd.DataFrame({'a': [], 'b': [], 'c': []}) | ||
| | @@ -1763,3 +1779,20 @@ def test_merge_equal_cat_dtypes2(): | |
| | ||
| # Categorical is unordered, so don't check ordering. | ||
| tm.assert_frame_equal(result, expected, check_categorical=False) | ||
| | ||
| | ||
| @pytest.mark.parametrize('merge_type', ['left_on', 'right_on']) | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. move this near the test_validation test | ||
| def test_missing_on_raises(merge_type): | ||
| # GH26824 | ||
| df1 = DataFrame({ | ||
| Contributor There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. call these left & right | ||
| 'A': [1, 2, 3, 4, 5, 6], | ||
| 'B': ['P', 'Q', 'R', 'S', 'T', 'U'] | ||
| }) | ||
| df2 = DataFrame({ | ||
| 'A': [1, 2, 4, 5, 7, 8], | ||
| 'C': ['L', 'M', 'N', 'O', 'P', 'Q'] | ||
| }) | ||
| msg = 'must equal' | ||
| kwargs = {merge_type: 'A'} | ||
| with pytest.raises(ValueError, match=msg): | ||
| pd.merge(df1, df2, how='left', **kwargs) | ||
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.
can you elaborate slightly; this only matters if
onitself is not provided. Do we have a check ifonis provide and either ofleft_onorright_onis notNone?