-
Notifications
You must be signed in to change notification settings - Fork 224
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
Change the line length limit of docstrings to 88 characters #962
Comments
To me, the example docstring looks good in VSCode and PyCharm but the return between 'separate'/'tick-intervals' and 'or'/' |
Oh, sorry for the confusion. To test what long docstrings look like, I changed (locally) the docstrings of @kwargs_to_strings(R="sequence", c="sequence_comma", p="sequence")
def basemap(self, **kwargs):
r"""
Plot base maps and frames for the figure.
Creates a basic or fancy basemap with axes, fill, and titles. Several map projections are available, and the user
may specify separate
tick-mark intervals for boundary annotation, ticking, and [optionally]
gridlines. A simple map scale or directional rose may also be plotted.
At least one of the parameters ``frame``, ``map_scale``, ``rose`` or
``compass`` must be specified. So it seems VSCode and PyCharm remove all "returns" in the docstrings, while Python and notebook keep them. |
Ping @GenericMappingTools/python @GenericMappingTools/python-maintainers for thoughts. |
I have no problems with changing it. Is your preference 88 or ~120 characters @seisman? |
We will stick with 88 characters (the default value of |
Would be fine for me. |
I would keep it at 79 characters. The point made at #384 and fatiando/verde#177 is for the Parameters section. E.g. in Wrapping to 88 char is fine on jupyterlab but not the classic jupyter notebook: |
Hmmm, I didn't know the "Shift+Tab" shortcut for the docstring tooltip until today. It seems JupyterLab wraps at 90 characters, but Jupyter Notebook wraps at 80 characters. So, the only reason to keep 79 characters is to make it look better in the classic Jupyter Notebook? But, is jupyter notebook still heavily used by people? Even the jupyter notebook project encourages people to use JupyterLab instead.
|
Wow, you've missed out on so much then!!
You will be surprised at how many people still use the classic notebook, even as jupyerlab has been around for a few years. I haven't had time to look, but Google Collaboratory, Kaggle and others probably use a similar classic notebook interface. This is one of those things where it's better to keep things as it is - if it ain't broken, don't fix it. |
Fair enough. Let's keep the docstring to 79 characters. |
The main reasons that I prefer to set the line-length limit of docstrings to 88 characters:
|
@seisman does the update handle indentation in Parameters blocks? That's still the worst part. And my experience is also that loads of people still use the classic notebook and have no intention of going to Jupyter Lab. |
Quickly checked the line-length in numpy, xarray and pandas. It seems numpy strictly follows the 80-char rule, while in xarray and pandas, most lines are shorter than 80-char, but there are also many lines longer than 80-char. |
Not sure why I had Here is the docstrings that I have changed for testing: lakes : str or list
*fill*\ [**+l**\|\ **+r**].
Set the shade, color, or pattern for lakes and river-lakes. The default is the fill chosen for wet areas set by the ``water``
parameter. Optionally, specify separate fills by appending
**+l** for lakes or **+r** for river-lakes, and passing multiple
strings in a list. Here is how the long line is wrapped in Jupyter Notebook v6.1.6: Since Notebook v6.1.6 was released three years ago on Dec, 24, 2020, I think it's OK to assume that users are using notebook>=6.1, and then it's safe to set the length limit to 88-characters. |
I'm ok with moving to 88-characters, if only to simplify the linter configuration (e.g. with #2909). But I'm not keen on re-wrapping all the docstrings from 79 to 88 characters and making lots of big diffs. We can enforce a line-length of 88, but only apply it for newly written documentation perhaps? |
I don't think we can. The line length of docstrings are controlled by Lines 86 to 87 in 013014b
If we change them to 88, all docstrings will be reformatted. |
Well, as long as it's automated by |
There must be some misunderstandings here. Currently, the docstrings are formatted by As in #2909, |
Ah yes, we're talking about the docstrings here, so |
It's a known issue https://docformatter.readthedocs.io/en/latest/faq.html. |
We use the Black tool to format the code, and the default line length of Black is 88 characters. The 88-char line length limit is only applied to code, not docstrings. For docstrings, the line length limit is 79 characters, and we use several tools, blackdoc, docformatter and flake8 to check and format the docstrings.
Then, the question is, why do we use different line length limits for code and docstrings?
We started to adopt the 79-char line length limit for docstrings in PR #384. The PR description also explains why we choose 79-char length (also see some external discussions in fatiando/verde#177)
Is the reason still valid?
I just changed one line of the docstrings of
Figure.basemap()
to ~120 chars, and checked what the docstrings look like in Jupyter, IPython, PyCharm, and VS Code. Here are the results:In Jupyter Notebook, the long docstrings look good with the
help()
function:Also look good in the Notebook Contextual Help page:
In a Python or IPython console:
In VS Code:
In PyCharm:
From the above screenshots, it's clear that these consoles, editors, and IDEs don't wrap docstrings at 80 characters and work well with long docstrings.
Now the questions are:
The text was updated successfully, but these errors were encountered: