-
-
Notifications
You must be signed in to change notification settings - Fork 18.5k
DOC: split docstring into multiple lines in excel.py #14073
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 1 commit
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 |
---|---|---|
|
@@ -29,6 +29,7 @@ | |
from warnings import warn | ||
from distutils.version import LooseVersion | ||
from pandas.util.decorators import Appender | ||
from textwrap import fill | ||
|
||
__all__ = ["read_excel", "ExcelWriter", "ExcelFile"] | ||
|
||
|
@@ -97,7 +98,7 @@ | |
na_values : scalar, str, list-like, or dict, default None | ||
Additional strings to recognize as NA/NaN. If dict passed, specific | ||
per-column NA values. By default the following values are interpreted | ||
as NaN: '""" + "', '".join(sorted(_NA_VALUES)) + """'. | ||
as NaN: '""" + fill("', '".join(sorted(_NA_VALUES)), 80) + """'. | ||
thousands : str, default None | ||
Thousands separator for parsing string columns to numeric. Note that | ||
this parameter is only necessary for columns stored as TEXT in Excel, | ||
|
@@ -168,7 +169,20 @@ def get_writer(engine_name): | |
raise ValueError("No Excel writer '%s'" % engine_name) | ||
|
||
|
||
@Appender(_read_excel_doc) | ||
def split_doc(doc_string, word_wrap=80): | ||
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.
But we will have to check some doc strings. 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. I think the how it is here done is fine. In principle we could add a check for this in Appender, but this is also fine. 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. no this is adding code which is the same as wrap 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. Yeah, moving it to a common location is better, I just meant the current function (after moving) is fine. wrap is not doing exactly the same as this does not take account of the indentation needed in this case 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. ok but moving to Appender is pretty trivially and he obvious location |
||
doc_lines = doc_string.split('\n') | ||
doc_string = '' | ||
for line in doc_lines: | ||
if len(line) > word_wrap: | ||
break_pos = line[:word_wrap].rfind(' ') | ||
indent_width = len(line) - len(line.lstrip()) | ||
line = line[:break_pos] + '\n' + \ | ||
' ' * indent_width + line[break_pos + 1:] | ||
doc_string += (line + '\n') | ||
return doc_string | ||
|
||
|
||
@Appender(split_doc(_read_excel_doc)) | ||
def read_excel(io, sheetname=0, header=0, skiprows=None, skip_footer=0, | ||
index_col=None, names=None, parse_cols=None, parse_dates=False, | ||
date_parser=None, na_values=None, thousands=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.
I would take a lower number than 80. We already have the indentation of 4 chars in the docstring itself, so maximum 76. But I would maybe take just 70 or so