ENH: add result_names argument to DataFrame.compare #44354 #47643
Conversation
|
Hello @ahmedibrhm! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found: There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻 Comment last updated at 2022-07-16 04:17:06 UTC |
phofl
left a comment
phofl
left a comment
There was a problem hiding this comment.
Can you add tests for invalid arguments? Wrong length, not a tuple, ...
pandas/core/shared_docs.py
Outdated
| If true, the result keeps values that are equal. | ||
| Otherwise, equal values are shown as NaNs. | ||
| suffixes : tuple, default ('self', 'other') |
There was a problem hiding this comment.
i think we should call this names or maybe result_names as suffixes implies we are appending these to something (ala merge)
There was a problem hiding this comment.
@jreback Sorry, I didn't see the comment.
I guess suffix should be used because to some extent the result will be merged. What I mean is that the comparison will be one block representing the different parts of the two dataframes.
There was a problem hiding this comment.
I can see your point but result names will mean that the result will have names. like we have an output which will have multiple names
There was a problem hiding this comment.
@jreback I edited the name of the argument
|
small comments, docs fail |
jreback
left a comment
jreback
left a comment
There was a problem hiding this comment.
let's discuss the name of this added arg
ahmedibrhm
changed the title
|
@phofl if any comments |
phofl
left a comment
phofl
left a comment
There was a problem hiding this comment.
Please add a test that raises because of an invalid input
|
@phofl done, added the test |
|
Thx @ahmedibrhm |
4 participants
This is following up on the pull request #44365
doc/source/whatsnew/v1.5.0.rstfile if fixing a bug or adding a new feature.