Correct assert_frame_equal doc string #22552
Conversation
Correct default values in assert_frame_equal
|
Codecov Report
@@ Coverage Diff @@
## master #22552 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 169 169
Lines 50787 50787
=======================================
Hits 46745 46745
Misses 4042 4042
Continue to review full report at Codecov.
|
pandas/util/testing.py
Outdated
| check_dtype : bool, default True | ||
| Whether to check the DataFrame dtype is identical. | ||
| check_index_type : bool / string {'equiv'}, default False | ||
| check_index_type : bool / string {'equiv'}, default 'equiv' |
There was a problem hiding this comment.
I think what we've been using in these cases is {'equiv'} or bool
There was a problem hiding this comment.
IMO that is more confusing. e.g. it might imply I pass a set. I'm not sure it's worth a special case for when there is only one possible string value.
There was a problem hiding this comment.
what is your suggestion, just 'equiv' or bool, default 'equiv' or something else? Besides being more consistent for the user, using the curly brackets in all cases would simplify parsing the types and adding validation and extracting stats. But if you are strongly in favor of not using them, I'm happy to merge this with it now, and see later on what's best when we implement that validation.
There was a problem hiding this comment.
I think as it is above is good. It's maybe a little verbose but is very clear.
check_index_type : bool / string {'equiv'}, default 'equiv'
Happy to revisit if a standard emerges.
There was a problem hiding this comment.
You have the standard here: https://pandas.pydata.org/pandas-docs/stable/contributing_docstring.html#parameter-types
If you can use the first format I suggested, that would be great.
There was a problem hiding this comment.
Thanks. I have used your suggestion.
datapythonista
added
the
Testing
|
Hello @pablojim! Thanks for updating the PR. Cheers ! There are no PEP8 issues in this Pull Request. 🍻 Comment last updated on September 03, 2018 at 11:52 Hours UTC |
pandas/util/testing.py
Outdated
| check_dtype : bool, default True | ||
| Whether to check the DataFrame dtype is identical. | ||
| check_index_type : bool / string {'equiv'}, default False | ||
| check_index_type : bool / string {'equiv'}, default 'equiv' |
There was a problem hiding this comment.
what is your suggestion, just 'equiv' or bool, default 'equiv' or something else? Besides being more consistent for the user, using the curly brackets in all cases would simplify parsing the types and adding validation and extracting stats. But if you are strongly in favor of not using them, I'm happy to merge this with it now, and see later on what's best when we implement that validation.
pandas/util/testing.py
Outdated
| See Also | ||
| -------- | ||
| assert_series_equal: equivalent method for asserting Series equality | ||
| DataFrame.equals: check DataFrame equality |
There was a problem hiding this comment.
Do you think it makes sense adding a example?
There was a problem hiding this comment.
I'm not sure an example is particularly useful in this case.
There was a problem hiding this comment.
We've got users of all levels, and things like comparing dataframes with different types are worth showing with examples.
Besides that, can you add a space before the colons in the see also section, capitalize the first letter of the description, and finish the description with a period? If you generate the html ./doc/make.py html --single pandas.util.testing.assert_frame_equal, I don't think without the space this is being rendered correcty.
There was a problem hiding this comment.
Added examples and corrected formatting as mentioned.
pandas/util/testing.py
Outdated
| See Also | ||
| -------- | ||
| assert_series_equal: equivalent method for asserting Series equality | ||
| DataFrame.equals: check DataFrame equality |
There was a problem hiding this comment.
We've got users of all levels, and things like comparing dataframes with different types are worth showing with examples.
Besides that, can you add a space before the colons in the see also section, capitalize the first letter of the description, and finish the description with a period? If you generate the html ./doc/make.py html --single pandas.util.testing.assert_frame_equal, I don't think without the space this is being rendered correcty.
Correct default values in assert_frame_equal doc string