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, BUG: Fix error in heading remapping for custom scipy.optimize:function domain directive #15443

Merged
merged 3 commits into from Mar 25, 2022

Conversation

rossbar
Copy link
Contributor

@rossbar rossbar commented Jan 21, 2022

There is a bug in .. scipy.optimize::function that causes the heading under the Returns section of the docstrings to be replaced with a heading with the wrong length. The numpydoc 1.2rc1 release candidate will raise a UserWarning when there is an incorrect header length. Because of the way the doc build is configured, this UserWarning is elevated to an exception that terminates the scipy doc build process, so this fix is necessary to build the scipy docs with numpydoc 1.2.

I also had to add an extra warnings filter for a distutils deprecation warning to get the docs to build locally. Not sure if this is the desired fix, but I thought I'd include it here just in case. I'm happy to drop 6ebf6e3 if there is a different preferred solution, or if it's preferred to be handled in a different PR.

@brandondavid brandondavid added Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.optimize defect A clear bug or issue that prevents SciPy from being installed or used as expected labels Feb 11, 2022
Fixes a bug that replaces the Returns heading with an
incorrect heading size in the generated docstring. This will
cause warnings (treated as errors) with numpydoc 1.2.
@rossbar
Copy link
Contributor Author

rossbar commented Mar 25, 2022

Okay, now that #15789 is in I've rebased and removed the numpydoc upper bound pins as discussed in the other PR. Now the passing doc builds in CI are indicative that the fix in a2a9a67 gets things working with the latest numpydoc (1.2).

AFAICT the CI failures are unrelated.

A quick summary of this PR:

  • a2a9a67: fix a bug in a custom scipy sphinx directive to get the scipy doc build working w/ numpydoc 1.2
  • afe165f: Add an additional warnings filter to make the doc build work w/ Python 3.10 (this is just something I noticed locally - could be moved to a separate PR, just LMK)
  • a5f39ab removes the ==1.1 pins from numpydoc

@tylerjereddy tylerjereddy added this to the 1.9.0 milestone Mar 25, 2022
Copy link
Contributor

@tylerjereddy tylerjereddy left a comment

Choose a reason for hiding this comment

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

Agreed that the CI failures are not related. The changes here all look sensible and the unpinning here already had a "thumbs up" from a few core devs in the related PR.

I'm happy to trust your expertise on the changes in doc/source/scipyoptdoc.py, which is the only thing that looks a bit tricky here. I did a spot check on some of the optimize docs in the CI-generated docs artifact and they seem "ok," though the fact that the docs build successfully with latest numpydoc is likely a better indicator than my visual inspection anyway.

@tylerjereddy tylerjereddy merged commit fd252cb into scipy:main Mar 25, 2022
@tylerjereddy
Copy link
Contributor

Thanks Ross

@@ -82,7 +82,7 @@ steps:
- script: pip install pytest-cov coverage codecov
displayName: 'Install coverage dependencies'
- ${{ if eq(parameters.refguide_check, true) }}:
- script: pip install matplotlib sphinx==3.1 numpydoc==1.1 "Jinja2<=3.0.3"
- script: pip install matplotlib sphinx==3.1 numpydoc "Jinja2<=3.0.3"
Copy link
Member

Choose a reason for hiding this comment

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

Ark it would have been good to test if we could then remove the pin on Jinja2. This is why I pinged you on the other issue.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

My bad, I missed this somehow "/. FWIW I tried this locally without the Jinja pin and everything worked.

@tupui tupui mentioned this pull request Mar 29, 2022
AnirudhDagar added a commit to AnirudhDagar/scipy that referenced this pull request Apr 29, 2022
This is just a cleanup for numpydoc that was left
after already being unpinned in scipy#15443
This is unrelated to the current PR.
@rgommers rgommers added the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label May 20, 2022
@tylerjereddy
Copy link
Contributor

This was either already in the release branch or recently merged in, so stripping the backport label.

@tylerjereddy tylerjereddy removed the backport-candidate This fix should be ported by a maintainer to previous SciPy versions. label Jun 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
defect A clear bug or issue that prevents SciPy from being installed or used as expected Documentation Issues related to the SciPy documentation. Also check https://github.com/scipy/scipy.org scipy.optimize
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants