Skip to content
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

Ensure there's a blank line between the class __doc__ and "Parameters" in build_doc docstrings #6004

Merged
merged 2 commits into from Sep 27, 2021

Conversation

jwodder
Copy link
Member

@jwodder jwodder commented Sep 23, 2021

Without this change, applying @build_doc to a class whose docstring doesn't end in a newline, like so:

@build_doc
class Foo(Interface):
     """Foo all the bars"""

    @staticmethod
    @eval_results
    def __call__():
        pass

will result in __call__ getting a docstring that starts out with:

Foo all the bars
Parameters
----------

and the lack of a newline between the class docstring and "Parameters" will result in Sphinx producing a warning when trying to build the documentation for the class. This PR fixes this behavior by ensuring that, if the class docstring doesn't already end with a newline, one is added, so that there are always at least two newlines (i.e., one blank line) before the "Parameters" header.

@jwodder jwodder added the semver-patch Increment the patch version when merged label Sep 23, 2021
@yarikoptic
Copy link
Member

would you be so kind to rebase against maint? I think we could even try to release since there were a few fixes (we are at 0.15.0-14-geb7a43b09) and we must not grow changelog for auto to succeed.

@jwodder jwodder changed the base branch from master to maint September 23, 2021 19:13
@yarikoptic
Copy link
Member

also if you could add a basic test to datalad/interface/tests/test_base.py which would demonstrate current issue (and test the fix) so we do not undo it -- would be great

@yarikoptic yarikoptic added the release Create a release when this pr is merged label Sep 23, 2021
@yarikoptic
Copy link
Member

Let's wait for the tests to pass, and then we could proceed with merge.

@codecov
Copy link

codecov bot commented Sep 23, 2021

Codecov Report

Merging #6004 (dd1154b) into maint (eb7a43b) will decrease coverage by 1.76%.
The diff coverage is 91.66%.

❗ Current head dd1154b differs from pull request most recent head 191233d. Consider uploading reports for the commit 191233d to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##            maint    #6004      +/-   ##
==========================================
- Coverage   89.68%   87.91%   -1.77%     
==========================================
  Files         308      305       -3     
  Lines       42115    42093      -22     
==========================================
- Hits        37770    37006     -764     
- Misses       4345     5087     +742     
Impacted Files Coverage Δ
datalad/interface/tests/test_base.py 98.36% <83.33%> (-0.78%) ⬇️
datalad/distribution/dataset.py 96.63% <100.00%> (+2.21%) ⬆️
datalad/downloaders/base.py 78.16% <100.00%> (+0.35%) ⬆️
datalad/interface/base.py 90.28% <100.00%> (+0.05%) ⬆️
datalad/plugin/wtf.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/addurls.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/no_annex.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/add_readme.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/plugin/check_dates.py 0.00% <0.00%> (-100.00%) ⬇️
datalad/support/tests/utils.py 0.00% <0.00%> (-100.00%) ⬇️
... and 98 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update eb7a43b...191233d. Read the comment docs.

@yarikoptic
Copy link
Member

Test seems to fail (didn't analyze it always)

FAIL: datalad.interface.tests.test_base.test_update_docstring_with_parameters_single_line_prefix
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/tmp/dl-miniconda-q0b2i3i0/lib/python3.9/site-packages/nose/case.py", line 198, in runTest
    self.test(*self.arg)
  File "/tmp/dl-miniconda-q0b2i3i0/lib/python3.9/site-packages/datalad/interface/tests/test_base.py", line 230, in test_update_docstring_with_parameters_single_line_prefix
    assert_in("This is a single line.\n\nParameters\n", fn.__doc__)
AssertionError: 'This is a single line.\n\nParameters\n' not found in 'This is a single line.'

@yarikoptic yarikoptic removed the release Create a release when this pr is merged label Sep 24, 2021
@jwodder
Copy link
Member Author

jwodder commented Sep 24, 2021

@yarikoptic The test should be fixed now. It turns out update_docstring_with_parameters() doesn't add a "Parameters" block if the function has only one argument, presumably based on the assumption that the argument will always be self. Might want to change that.

@yarikoptic
Copy link
Member

crawler fail is likely unrelated. one run of travis -- inability to download conda, unlikely to bring surprises, but I restarted the job -- better be safe than sorry ;) if would turn green -- I think we should proceed with merge. Thank you @jwodder

@yarikoptic yarikoptic added this to the 0.15.2 milestone Sep 27, 2021
@yarikoptic
Copy link
Member

Travis is happy now

@yarikoptic yarikoptic merged commit 003c7b8 into datalad:maint Sep 27, 2021
@jwodder jwodder deleted the doc-blank-line branch October 12, 2021 02:45
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
semver-patch Increment the patch version when merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants