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

Fix documentation build main #3799

Merged
merged 11 commits into from
Jan 9, 2023
Merged

Conversation

beroda
Copy link
Contributor

@beroda beroda commented Jan 9, 2023

This is an (another and hopefully last) attempt to resolve #3782

@github-actions github-actions bot added documentation Anything related to the documentation/website maintenance Low-impact maintenance activity labels Jan 9, 2023
@codecov
Copy link

codecov bot commented Jan 9, 2023

Codecov Report

Merging #3799 (47fe743) into main (bda7d71) will increase coverage by 0.02%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main    #3799      +/-   ##
==========================================
+ Coverage   94.15%   94.18%   +0.02%     
==========================================
  Files          86       86              
  Lines       18891    18899       +8     
==========================================
+ Hits        17787    17800      +13     
+ Misses       1104     1099       -5     

@MatthewFlamm
Copy link
Contributor

We should merge #3800, if it works, first. This will allow us to check that this PR indeed fixes the issue.

@beroda
Copy link
Contributor Author

beroda commented Jan 9, 2023

@MatthewFlamm Thank you !

@MatthewFlamm MatthewFlamm added the full-doc Run full documentation build on CI label Jan 9, 2023
Co-authored-by: MatthewFlamm <39341281+MatthewFlamm@users.noreply.github.com>
@beroda
Copy link
Contributor Author

beroda commented Jan 9, 2023

I think the should finish with a period warning at the end of a parameter's description (as reported here) should be captured by pydocstyle and not by the full documentation build (which is performed very rarely by devs).

But unfortunately I was not able to find any error code associated with that http://www.pydocstyle.org/en/stable/error_codes.html 😢

Copy link
Member

@banesullivan banesullivan left a comment

Choose a reason for hiding this comment

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

Recommend merging to fix the build on main.

We can figure out how to handle those warnings with pydicstyle later

Copy link
Contributor

@MatthewFlamm MatthewFlamm left a comment

Choose a reason for hiding this comment

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

LGTM

@beroda beroda marked this pull request as ready for review January 9, 2023 20:51
@MatthewFlamm
Copy link
Contributor

MatthewFlamm commented Jan 9, 2023

I think the should finish with a period warning at the end of a parameter's description (as reported here) should be captured by pydocstyle and not by the full documentation build (which is performed very rarely by devs).

But unfortunately I was not able to find any error code associated with that http://www.pydocstyle.org/en/stable/error_codes.html 😢

I believe pydocstyle is mostly checker for pep 257, which is much less prescriptive than numpydoc. In fact, pep 257 has examples with parameter descriptions that do not end in a period. See https://peps.python.org/pep-0257/.

For numpydoc, we could try https://numpydoc.readthedocs.io/en/latest/validation.html#docstring-validation-using-python.

Edit: Annoyingly, to use this, you have to specify every single function that you want to check. You cannot specify a package or module. Running python -m numpydoc pyvista.Cell.copy_meta_from using current main branch does yield an error for {'**kwargs', '*args'}, but it also returns error for things we have ignored for the documentation check in conf.py. This seems like something that is better requested to be part of numpydoc.

More edits: It looks like the numpydoc validation originated from Pandas. Pandas maintains a script for scraping public api function docstrings here and then customizing numpydoc validation: https://github.com/pandas-dev/pandas/blob/main/scripts/validate_docstrings.py. Instead of re-implementing here, it would be preferable if again, Pandas merged the public api portion of the code into numpydoc.

@banesullivan banesullivan merged commit 01a9140 into pyvista:main Jan 9, 2023
@beroda beroda deleted the maint/doc-errors branch January 9, 2023 22:20
@banesullivan banesullivan mentioned this pull request Feb 1, 2023
5 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Anything related to the documentation/website full-doc Run full documentation build on CI maintenance Low-impact maintenance activity
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Documentation build is failing on main
3 participants