-
-
Notifications
You must be signed in to change notification settings - Fork 18.2k
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
DOC: Fix DataFrame.to_csv docstring and add an example. GH22459 #22475
Conversation
Travis fails due to a curl error. |
pandas/core/generic.py
Outdated
encoding : string, optional | ||
A string representing the encoding to use in the output file, | ||
defaults to 'ascii' on Python 2 and 'utf-8' on Python 3. | ||
compression : {'infer', 'gzip', 'bz2', 'zip', 'xz', None}, |
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.
Think the domain of potential values should remain here - was this causing some type of validation error?
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.
In fact the length of the potential values plus the default value span on two lines with the 80 characters limit. This leads to the following error:
Parameter "compression" description should start with a capital letter
.
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.
Do you mind changing the parameter types, so they are Python types when possible (str
, bool
, int
...) please? In case of list you can use something like list of str
. The default can stay at the end, but things like that the length of a string should be 1, should go into the description.
pandas/core/generic.py
Outdated
Examples | ||
-------- | ||
|
||
>>> df = pd.DataFrame({'col1': [1], 'col2': ['a'], 'col3': [10.1]}) |
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 make the example a bit more meaningful? So, data looks real, and is easier to understand.
Also, can you remove the blank line before this line?
pandas/core/generic.py
Outdated
|
||
See Also | ||
-------- | ||
pandas.read_csv : load a CSV file into a DataFrame |
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 also add DataFrame.to_excel
?
And make the See Also
description start with capital and finish with period.
…section to DataFrame.to_csv docstring
Codecov Report
@@ Coverage Diff @@
## master #22475 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 169 169
Lines 50787 50787
=======================================
Hits 46745 46745
Misses 4042 4042
Continue to review full report at Codecov.
|
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.
Looks great, just two small things.
pandas/core/generic.py
Outdated
The order of arguments for Series was changed. | ||
Returns | ||
------- | ||
If path_or_buf is None, returns the resulting csv format as a string. |
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 add a line first None or str
and then this description in the next line indented
pandas/core/generic.py
Outdated
Examples | ||
-------- | ||
>>> df = pd.DataFrame({'name': ['Raphael', 'Donatello'], | ||
... 'mask': ['red', 'purple'], 'weapon': ['sai', 'bo staff']}) |
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 indent mask
at the level of name
and weapon
in the next line with the same indentation
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.
Looks great, thanks for fixing the docsting @Moisan
circleci |
@Moisan do you mind updating you branch with master (i.e. The fail in the build is unrelated to your changes, but doesn't seem it reruns when I trigger it from the web interface. |
@WillAyd please merge when you're happy with this |
Thanks @Moisan |
BTW, sphinx requires a blank line before a |
git diff upstream/master -u -- "*.py" | flake8 --diff
Fix the DataFrame.to_csv docstring to match
scripts/validate_docstrings.py
as explained in #22459. I also added an example. Is the whatsnew entry needed for documentation too?