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

DOC: Attempt at autoformatting docstrings in morphology/* #5317

Merged
merged 1 commit into from
Apr 9, 2021

Conversation

Carreau
Copy link
Contributor

@Carreau Carreau commented Apr 7, 2021

Numpydoc is specific about the meaning of spaces arround : and a few
other things, while it may look almost identical in sphinx depending on
themes it is semantically different.

I'm attempting to write an docstring autoreformatter thetry to fix
common mistakes, and fix a couple of other things like typo in parameter
names, remove parameters that are still documented, and try to get a
uniform style.

You'll see in the diff

  • add / removal of spaces
  • add / removal of blank lines
  • indent 4 spaces always.
  • Capitalisation of section names
  • removal of border_value parameter which is indeed not a function
    argument.

In morphology/* this hasn't found any other issue it can fix, and the
only other non-automatically fixable issues are missing documentation
for dtype parameters.

For reviewers

  • Check that the PR title is short, concise, and will make sense 1 year
    later.
  • Check that new functions are imported in corresponding __init__.py.
  • Check that new features, API changes, and deprecations are mentioned in
    doc/release/release_dev.rst.

@sciunto sciunto added the 📄 type: Documentation Updates, fixes and additions to documentation label Apr 7, 2021
@sciunto
Copy link
Member

sciunto commented Apr 7, 2021

Great! Thank you for trying on our source files. The failure is related to #5318

Copy link
Contributor

@grlee77 grlee77 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is great, thanks! I commented on one inserted space that looked strange to me.

For context, I think these were generated with @Carreau's velin

@@ -155,6 +155,7 @@ def convex_hull_object(image, *, connectivity=2):
----------
image : (M, N) ndarray
Binary input image.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this extra space is unexpected?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I interpreted it as a convention to split between arguments before * and after. Just a guess that I found elegant.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't believe there are written rules about this; wether to put blank lines between items varies across projects; currently my auto formatter has some heuristics, in particular if one items description have multiple paragraph (which is the case here for connectivity) it will add one line between all items.

It is quite hard to no modify whitespace as is does Docstring -> AST-Like repr -> back to text, tso initial whitespace information is lost.

Numpydoc is specific about the meaning of spaces arround : and a few
other things, while it may look almost identical in sphinx depending on
themes it is semantically different.

I'm attempting to write an docstring autoreformatter thetry to fix
common mistakes, and fix a couple of other things like typo in parameter
names, remove parameters that are still documented, and try to get a
uniform style.

You'll see in the diff
 - add / removal of spaces
 - add / removal of blank lines
 - indent 4 spaces always.
 - Capitalisation of section names
 - removal of `border_value` parameter which is indeed not a function
    argument.

In morphology/* this hasn't found any other issue it can fix, and the
only other non-automatically fixable issues are missing documentation
for `dtype` parameters.
@Carreau
Copy link
Contributor Author

Carreau commented Apr 8, 2021

I reran it with the --compact option to keep blank line to a minimal, this removed the line you pointed to.

@grlee77 grlee77 merged commit 3725d7e into scikit-image:main Apr 9, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
📄 type: Documentation Updates, fixes and additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants