DOC/ERR: better error message on no common merge keys #19427

swyoon · 2018-01-28T08:30:26Z

[o] closes DOC/ERR: better error message on no common merge keys #19391
[o] tests passed
[o] passes git diff upstream/master -u -- "*.py" | flake8 --diff
Let MergeError emit values keyword arguments
Add DataFrame.join on See also section of pandas.merge

swyoon · 2018-01-28T09:39:48Z

It seems that the failed test has nothing to do with the content of the commit though..

codecov · 2018-01-30T17:42:53Z

Codecov Report

Merging #19427 into master will decrease coverage by 1.63%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master   #19427      +/-   ##
==========================================
- Coverage    91.6%   89.96%   -1.64%     
==========================================
  Files         150      150              
  Lines       48750    48726      -24     
==========================================
- Hits        44657    43837     -820     
- Misses       4093     4889     +796

Flag	Coverage Δ
#multiple	`89.96% <100%> (-0.01%)`	⬇️
#single	`?`

Impacted Files	Coverage Δ
pandas/core/frame.py	`97.42% <ø> (-0.01%)`	⬇️
pandas/core/reshape/merge.py	`94.21% <100%> (-0.01%)`	⬇️
pandas/core/computation/pytables.py	`62.5% <0%> (-29.88%)`	⬇️
pandas/io/pytables.py	`64.17% <0%> (-28.28%)`	⬇️
pandas/io/feather_format.py	`71.42% <0%> (-14.29%)`	⬇️
pandas/core/computation/common.py	`78.57% <0%> (-7.15%)`	⬇️
pandas/core/computation/expr.py	`84.27% <0%> (-4.39%)`	⬇️
pandas/io/formats/console.py	`66.66% <0%> (-3.04%)`	⬇️
pandas/io/clipboard/clipboards.py	`29.88% <0%> (-2.3%)`	⬇️
pandas/io/formats/printing.py	`87.61% <0%> (-1.77%)`	⬇️
... and 35 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 54f1b3e...e4c5b58. Read the comment docs.

TomAugspurger · 2018-01-30T19:51:54Z

Can you add a test for this, and a release note?

What's the output look like for large DataFrames? Does the repr truncation work correctly?

TomAugspurger · 2018-01-30T19:52:43Z

pandas/core/frame.py

@@ -233,7 +233,7 @@
 --------
 merge_ordered
 merge_asof
-
+DataFrame.join : For index-based merge operations


I think you can remove the explanation, everything other than the name. I'm not sure what numpydoc does with it, but those are turned into hyperlinks for the HTML docs.

swyoon · 2018-01-31T13:45:09Z

Release note added, see also modified.
I believe my contribution is about the error message. Should the error message be included in the test?
It seems that there already exists a test for a merge with no common keys.
I don't get what you said about the large DataFrames. Could you explain a bit more?

TomAugspurger · 2018-01-31T13:51:01Z

I believe my contribution is about the error message. Should the error message be included in the test?

Yes, see pandas.util.testing.assert_raises_regex for how to do it. You'll want to do a few different tests and assert that the correct information is always in the exception message.

Ignore my comment about large dataframes I think.

TomAugspurger · 2018-01-31T13:51:37Z

pandas/core/reshape/merge.py

@@ -1028,7 +1028,14 @@ def _validate_specification(self):
                common_cols = self.left.columns.intersection(
                    self.right.columns)
                if len(common_cols) == 0:
-                    raise MergeError('No common columns to perform merge on')
+                    raise MergeError('No common columns to perform merge on. '
+                                     'Merge options: right_on={ron}, '


Can you change the order to be left_on, right_on, left_index, right_index, to match the order in the signature?

Oh sure I can. will do it.

to make this more readable you can break it like

raise MergeError( 'No common ......' )

jreback · 2018-02-01T11:39:34Z

pandas/core/reshape/merge.py

@@ -1028,7 +1028,14 @@ def _validate_specification(self):
                common_cols = self.left.columns.intersection(
                    self.right.columns)
                if len(common_cols) == 0:
-                    raise MergeError('No common columns to perform merge on')
+                    raise MergeError('No common columns to perform merge on. '
+                                     'Merge options: right_on={ron}, '


to make this more readable you can break it like

raise MergeError( 'No common ......' )

jreback · 2018-02-01T11:40:06Z

pandas/core/reshape/merge.py

+                                     'left_on={lon}, right_index={ridx}, '
+                                     'left_index={lidx}'
+                                     .format(ron=self.right_on,
+                                             lon=self.left_on,


can you update the test for this (it prob just raises) and match the error message with assert_raises_regex

jreback · 2018-02-06T11:15:07Z

can you rebase and update

swyoon · 2018-02-06T13:06:16Z

The line break and the order of options are modified.
Added a check for error message by assert_raises_regex.

- Let MergeError emit values keyword arguments. - Add DataFrame.join on 'See also' section of pandas.merge. - Add an item in whatsnew - Update test_no_overlap_more_informative_error to check the message from MergeError

jreback · 2018-02-06T14:16:21Z

thanks @swyoon

swyoon · 2018-02-06T14:59:48Z

thanks for kind reviews! @jreback @TomAugspurger

swyoon force-pushed the swyoon branch from 1af76b7 to 59f7a23 Compare January 30, 2018 16:55

TomAugspurger reviewed Jan 30, 2018

View reviewed changes

swyoon force-pushed the swyoon branch from 59f7a23 to 7e2d192 Compare January 31, 2018 12:09

TomAugspurger reviewed Jan 31, 2018

View reviewed changes

jreback added the Error Reporting Incorrect or improved errors from pandas label Feb 1, 2018

jreback requested changes Feb 1, 2018

View reviewed changes

DOC/ERR: better error message on no common merge keys

e4c5b58

- Let MergeError emit values keyword arguments. - Add DataFrame.join on 'See also' section of pandas.merge. - Add an item in whatsnew - Update test_no_overlap_more_informative_error to check the message from MergeError

swyoon force-pushed the swyoon branch from 7e2d192 to e4c5b58 Compare February 6, 2018 13:32

jreback added this to the 0.23.0 milestone Feb 6, 2018

jreback approved these changes Feb 6, 2018

View reviewed changes

jreback merged commit d5eead6 into pandas-dev:master Feb 6, 2018

swyoon deleted the swyoon branch February 6, 2018 14:59

harisbal pushed a commit to harisbal/pandas that referenced this pull request Feb 28, 2018

DOC/ERR: better error message on no common merge keys (pandas-dev#19427)

bc1cd6d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC/ERR: better error message on no common merge keys #19427

DOC/ERR: better error message on no common merge keys #19427

swyoon commented Jan 28, 2018

swyoon commented Jan 28, 2018 •

edited

Loading

codecov bot commented Jan 30, 2018 •

edited

Loading

TomAugspurger commented Jan 30, 2018

TomAugspurger Jan 30, 2018

swyoon commented Jan 31, 2018 •

edited

Loading

TomAugspurger commented Jan 31, 2018

TomAugspurger Jan 31, 2018

swyoon Jan 31, 2018

jreback Feb 1, 2018

jreback Feb 1, 2018

jreback Feb 1, 2018

jreback commented Feb 6, 2018

swyoon commented Feb 6, 2018 •

edited

Loading

jreback commented Feb 6, 2018

swyoon commented Feb 6, 2018

DOC/ERR: better error message on no common merge keys #19427

DOC/ERR: better error message on no common merge keys #19427

Conversation

swyoon commented Jan 28, 2018

swyoon commented Jan 28, 2018 • edited Loading

codecov bot commented Jan 30, 2018 • edited Loading

Codecov Report

TomAugspurger commented Jan 30, 2018

TomAugspurger Jan 30, 2018

Choose a reason for hiding this comment

swyoon commented Jan 31, 2018 • edited Loading

TomAugspurger commented Jan 31, 2018

TomAugspurger Jan 31, 2018

Choose a reason for hiding this comment

swyoon Jan 31, 2018

Choose a reason for hiding this comment

jreback Feb 1, 2018

Choose a reason for hiding this comment

jreback Feb 1, 2018

Choose a reason for hiding this comment

jreback Feb 1, 2018

Choose a reason for hiding this comment

jreback commented Feb 6, 2018

swyoon commented Feb 6, 2018 • edited Loading

jreback commented Feb 6, 2018

swyoon commented Feb 6, 2018

swyoon commented Jan 28, 2018 •

edited

Loading

codecov bot commented Jan 30, 2018 •

edited

Loading

swyoon commented Jan 31, 2018 •

edited

Loading

swyoon commented Feb 6, 2018 •

edited

Loading