Add docformatter to format plain text in docstrings #642

seisman · 2020-10-04T21:19:36Z

Description of proposed changes

Add docformatter to format plaintext in docstrings, as mentioned in #497.

docformatter follows PEP 257, which is compatible with numpydoc style.

EDIT: There may be some incompatibility. The new documentation will look messy if --pre-summary-newline is not used.

Preview the documentation: https://pygmt-git-docstring-formatter.gmt.vercel.app/

Fixes #497.

Reminders

Run make format and make check to make sure the code follows the style guide.
Add tests for new features or tests that would have caught the bug that you're fixing.
Add new public functions/methods/classes to doc/api/index.rst.
Write detailed docstrings for all functions/methods.
If adding new functionality, add an example to docstrings or tutorials.

Makefile

pygmt/_version.py

pygmt/tests/test_grdcut.py

weiji14

Mixed feelings about using docformatter here. There are some aspects of PEP 257 that are nice, while others seem strange (though it might just require some getting used to). As you mentioned too at #642 (comment), the version of docformatter (1.3.1) from PyPI is somewhat out of date (11 Nov 2019) and there's features in the master branch which would be nice to have!

Also a bit wary about adding yet another lint tool into our toolbox since it might put off new contributors. The intention is good (making it easier to style code), but we'll need to remind ourselves to put code before formatting (as mentioned in https://github.com/GenericMappingTools/pygmt/blob/master/MAINTENANCE.md#reviewing-and-merging-pull-requests).

weiji14 · 2020-10-04T22:38:56Z

pygmt/clib/session.py

            Callback function that the GMT C API will use to print log and
-            error messages. We'll capture the messages and print them to stderr
-            so that they will show up on the Jupyter notebook.
+            error messages.
+
+            We'll capture the messages and print them to stderr so that they
+            will show up on the Jupyter notebook.


So docformatter splits things up here, the first sentence is a 'summary', and the other sentences become a 'description'. Interesting.

I think "summary" can be only one sentence, so the "summary" and "description" is separated at the first "period".

weiji14 · 2020-10-04T22:40:51Z

pygmt/sphinx_gallery.py

-        """
-        Called by sphinx-gallery to save the figures generated after running
-        code.
-        """
+        """Called by sphinx-gallery to save the figures generated after running
+        code."""


This looks funny too, why doesn't docformatter put a newline here? Seems to be raised before at PyCQA/docformatter#9.

Once PyCQA/docformatter#49 is merged (and a docformatter release is made), then we can rerender things and hopefully get a smaller diff.

Docformatter 1.4 has been released at https://pypi.org/project/docformatter/1.4/ so we can try to finish up this PR.

seisman · 2020-10-04T23:29:18Z

As you mentioned too at #642 (comment), the version of docformatter (1.3.1) from PyPI is somewhat out of date (11 Nov 2019) and there's features in the master branch which would be nice to have!

Perhaps we can ask if they could have a new release.

Also a bit wary about adding yet another lint tool into our toolbox since it might put off new contributors.

It seems straightforward to support slash commands like /format using slash-command-dispatch (see a demo at peter-evans/slash-command-dispatch#28). Then contributors and even core developer won't need to run make format locally.

weiji14 · 2020-10-05T00:05:14Z

Perhaps we can ask if they could have a new release.

It's been asked at PyCQA/docformatter#55.

It seems straightforward to support slash commands like /format using slash-command-dispatch (see a demo at peter-evans/slash-command-dispatch#28). Then contributors and even core developer won't need to run make format locally.

Ah that does look neat!

seisman · 2020-10-05T02:18:51Z

Perhaps we can ask if they could have a new release.

It's been asked at myint/docformatter#55.

Let's keep this PR open for a while.

weiji14 · 2021-01-20T05:28:50Z

Makefile

@@ -6,6 +6,8 @@ PYTEST_COV_ARGS=--cov=$(PROJECT) --cov-config=../pyproject.toml \
 			--pyargs ${PYTEST_EXTRA}
 BLACK_FILES=$(PROJECT) setup.py doc/conf.py examples
 BLACKDOC_OPTIONS=--line-length 79
+DOCFORMATTER_FILES=$(PROJECT) setup.py doc/conf.py examples
+DOCFORMATTER_OPTIONS=--recursive --pre-summary-newline --wrap-summaries 79 --wrap-descriptions 79


Suggested change

DOCFORMATTER_OPTIONS=--recursive --pre-summary-newline --wrap-summaries 79 --wrap-descriptions 79

DOCFORMATTER_OPTIONS=--recursive --pre-summary-newline --make-summary-multi-line --wrap-summaries 79 --wrap-descriptions 79

Do we want to force --make-summary-multi-line (see https://github.com/myint/docformatter/tree/v1.4#options)? This will turn every description (even short ones) into:

""" This is a description. """

Yes, it looks cleaner.

Just find that PEP 257 says:

Unless the entire docstring fits on a line, place the closing quotes on a line by themselves.

Hmm, seems like docformatter didn't implement that one-line docstring PEP 257 guideline correctly? I think it's ok to just force multi-line though, as long as there's an automated solution.

Co-authored-by: Wei Ji <[email protected]>

weiji14 · 2021-01-21T02:56:20Z

/format

weiji14

Just one nitpick on alphabetically ordering, otherwise should be good to merge once the Style Checks tests pass.

Edit: /format command at #642 (comment) didn't work because we need to install docformatter at this line too:

pygmt/.github/workflows/format-command.yml

Line 29 in 9a5858f

pip install black blackdoc flake8 isort

.github/workflows/ci_tests.yaml

CONTRIBUTING.md

Makefile

environment.yml

requirements-dev.txt

Co-authored-by: Wei Ji <[email protected]>

…dev.txt

seisman · 2021-01-21T03:44:05Z

/format

weiji14 · 2021-01-21T03:54:15Z

/format

Might need to do it manually locally for now, the Github Action is using the workflow yaml file from the master branch.

pygmt/tests/test_figure.py

…ols#642) * Add docformatter to format docstrings following a subset of PEP 257 * Add --make-summary-multi-line * Install docformatter to make /format work * Sort dependencies alphabetically in environment.yml and requirements-dev.txt * Format all docstrings Co-authored-by: Wei Ji <[email protected]>

seisman commented Oct 4, 2020

View reviewed changes

Makefile Outdated Show resolved Hide resolved

seisman marked this pull request as draft October 4, 2020 21:26

seisman force-pushed the docstring-formatter branch from bfbe503 to 3589c88 Compare October 4, 2020 21:57

vercel bot temporarily deployed to Preview October 4, 2020 21:57 Inactive

seisman commented Oct 4, 2020

View reviewed changes

pygmt/_version.py Outdated Show resolved Hide resolved

weiji14 reviewed Oct 4, 2020

View reviewed changes

pygmt/tests/test_grdcut.py Outdated Show resolved Hide resolved

vercel bot temporarily deployed to Preview October 4, 2020 22:20 Inactive

seisman marked this pull request as ready for review October 4, 2020 22:32

seisman added the maintenance Boring but important stuff for the core devs label Oct 4, 2020

seisman added this to the 0.2.1 milestone Oct 4, 2020

weiji14 reviewed Oct 4, 2020

View reviewed changes

seisman marked this pull request as draft October 5, 2020 02:18

weiji14 mentioned this pull request Oct 24, 2020

Switch from versioneer to setuptools-scm #671

Closed

seisman modified the milestones: 0.2.1, 0.3.0 Nov 6, 2020

seisman mentioned this pull request Nov 6, 2020

Wrap velo #525

Merged

seisman force-pushed the docstring-formatter branch from 096ac84 to 2557e8f Compare November 23, 2020 03:33

vercel bot temporarily deployed to Preview November 23, 2020 03:33 Inactive

vercel bot temporarily deployed to Preview December 16, 2020 07:39 Inactive

seisman force-pushed the docstring-formatter branch from a7b6f3d to d9822c2 Compare December 30, 2020 02:57

vercel bot temporarily deployed to Preview December 30, 2020 02:57 Inactive

seisman force-pushed the docstring-formatter branch from d9822c2 to 0fcfb24 Compare December 30, 2020 02:59

vercel bot temporarily deployed to Preview December 30, 2020 03:03 Inactive

seisman added 2 commits December 29, 2020 22:03

Add docformatter to format docstrings following a subset of PEP 257

17fe7a9

Fix a typo

54213f7

seisman force-pushed the docstring-formatter branch from 0fcfb24 to 54213f7 Compare December 30, 2020 03:04

vercel bot temporarily deployed to Preview December 30, 2020 03:09 Inactive

Merge branch 'master' into docstring-formatter

b1e6f37

vercel bot temporarily deployed to Preview January 8, 2021 21:05 Inactive

weiji14 reviewed Jan 20, 2021

View reviewed changes

Add --make-summary-multi-line

4d4e5bf

Co-authored-by: Wei Ji <[email protected]>

vercel bot temporarily deployed to Preview January 20, 2021 05:54 Inactive

weiji14 reviewed Jan 21, 2021

View reviewed changes

.github/workflows/ci_tests.yaml Outdated Show resolved Hide resolved

CONTRIBUTING.md Outdated Show resolved Hide resolved

Makefile Outdated Show resolved Hide resolved

environment.yml Show resolved Hide resolved

requirements-dev.txt Show resolved Hide resolved

Install docformatter to make /format work

18602d6

vercel bot temporarily deployed to Preview January 21, 2021 03:05 Inactive

Apply suggestions from code review

f4ae7ba

Co-authored-by: Wei Ji <[email protected]>

vercel bot temporarily deployed to Preview January 21, 2021 03:13 Inactive

seisman added 2 commits January 20, 2021 22:15

Sort dependencies alphabetically in environment.yml and requirements-…

09d1d5c

…dev.txt

Merge branch 'master' into docstring-formatter

c102784

vercel bot temporarily deployed to Preview January 21, 2021 03:19 Inactive

Format all docstrings

e6976d5

vercel bot temporarily deployed to Preview January 21, 2021 03:57 Inactive

weiji14 reviewed Jan 21, 2021

View reviewed changes

pygmt/tests/test_figure.py Outdated Show resolved Hide resolved

Fix typo

8988d66

vercel bot temporarily deployed to Preview January 21, 2021 04:06 Inactive

weiji14 approved these changes Jan 21, 2021

View reviewed changes

weiji14 marked this pull request as ready for review January 21, 2021 08:11

weiji14 merged commit bbd84d1 into master Jan 21, 2021

weiji14 deleted the docstring-formatter branch January 21, 2021 08:34

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add docformatter to format plain text in docstrings #642

Add docformatter to format plain text in docstrings #642

seisman commented Oct 4, 2020 •

edited

Loading

weiji14 left a comment

weiji14 Oct 4, 2020

seisman Oct 4, 2020

weiji14 Oct 4, 2020

weiji14 Nov 23, 2020

weiji14 Jan 20, 2021

seisman commented Oct 4, 2020

weiji14 commented Oct 5, 2020

seisman commented Oct 5, 2020

weiji14 Jan 20, 2021 •

edited

Loading

seisman Jan 20, 2021

seisman Jan 20, 2021

weiji14 Jan 21, 2021

weiji14 commented Jan 21, 2021

weiji14 left a comment •

edited

Loading

seisman commented Jan 21, 2021

weiji14 commented Jan 21, 2021

	DOCFORMATTER_OPTIONS=--recursive --pre-summary-newline --wrap-summaries 79 --wrap-descriptions 79
	DOCFORMATTER_OPTIONS=--recursive --pre-summary-newline --make-summary-multi-line --wrap-summaries 79 --wrap-descriptions 79

Add docformatter to format plain text in docstrings #642

Add docformatter to format plain text in docstrings #642

Conversation

seisman commented Oct 4, 2020 • edited Loading

weiji14 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seisman commented Oct 4, 2020

weiji14 commented Oct 5, 2020

seisman commented Oct 5, 2020

weiji14 Jan 20, 2021 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

weiji14 commented Jan 21, 2021

weiji14 left a comment • edited Loading

Choose a reason for hiding this comment

seisman commented Jan 21, 2021

weiji14 commented Jan 21, 2021

seisman commented Oct 4, 2020 •

edited

Loading

weiji14 Jan 20, 2021 •

edited

Loading

weiji14 left a comment •

edited

Loading