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

Whitespace change in autogenerated CONFIG docstrings causes Sphinx warnings #1191

Closed
lbianchi-lbl opened this issue May 25, 2023 · 5 comments
Assignees
Labels
backward-compat Affects backward compatibility bug Something isn't working documentation Documentations comments and requests Priority:High High Priority Issue or PR

Comments

@lbianchi-lbl
Copy link
Contributor

lbianchi-lbl commented May 25, 2023

Observed in watertap-org/watertap#1032 as part of the process to update dependencies from IDAES 2.0.0/Pyomo 6.5.0:

2023-05-25_16-17

The generated warning (one of many, relative to the class shown in the screenshot above) from this ReadTheDocs build log:

/home/docs/checkouts/readthedocs.org/user_builds/watertap/checkouts/1034/watertap/property_models/cryst_prop_pack.py:docstring of watertap.property_models.cryst_prop_pack.NaClParameterBlock:1: WARNING: Block quote ends without a blank line; unexpected unindent.

This causes the build to fail if the Sphinx -W/fail-on-warning setting is enabled.

@lbianchi-lbl lbianchi-lbl changed the title Whitespace change in autogenerated CONFIG docstrings break Sphinx Whitespace change in autogenerated CONFIG docstrings causes Sphinx warnings May 25, 2023
@lbianchi-lbl lbianchi-lbl added bug Something isn't working backward-compat Affects backward compatibility documentation Documentations comments and requests labels May 25, 2023
@lbianchi-lbl
Copy link
Contributor Author

lbianchi-lbl commented May 25, 2023

@andrewlee94
Copy link
Member

@jsiirola Any suggestion on this? Is it something that is in the IDAES usage of the Pyomo documentation functions (e.g. something we need to add to include the trailing linebreak), a difference in the Pyomo tool output, or a bad habit that Pyomo is now being more vocal about?

@lbianchi-lbl
Copy link
Contributor Author

@jsiirola Any suggestion on this? Is it something that is in the IDAES usage of the Pyomo documentation functions (e.g. something we need to add to include the trailing linebreak), a difference in the Pyomo tool output, or a bad habit that Pyomo is now being more vocal about?

I've just tried using https://pypi.org/project/rstcheck/, and it seems to confirm that the IDAES 2.0.0/Pyomo 6.5.0 output is valid, while the IDAES/idaes-pse@main/Pyomo 6.5.1 is not:

conda create --yes test-idaes-1191 python=3.10 && conda activate test-idaes-1191
pip install "watertap @ git+https://github.com/watertap-org/watertap@main" rstcheck
pip show pyomo idaes-pse
pip uninstall --yes pyomo idaes-pse && pip install idaes-pse==2.0.0 pyomo=6.5.0 && pip show pyomo idaes-pse
python -c 'from watertap.property_models.cryst_prop_pack import NaClParameterBlock as obj; print(obj.__doc__)' | rstcheck -
# output: Success! No issues detected.
pip uninstall --yes pyomo idaes-pse && pip install "idaes-pse @ git+https://github.com/IDAES/idaes-pse@main" && pip show pyomo idaes-pse
python -c 'from watertap.property_models.cryst_prop_pack import NaClParameterBlock as obj; print(obj.__doc__)' | rstcheck -
# output: <stdin>:17: (WARNING/2) Block quote ends without a blank line; unexpected unindent. Error! Issues detected.

@jsiirola
Copy link
Contributor

I looked into this and Pyomo/pyomo#2853 restores (most of) the old behavior. However, relying on the indentation in the ConfigDict declarations to generate correct RST is fragile at best. This is exactly why Pyomo introduced the use of wrap_reStructuredText() in the numpydoc formatter [it also automatically documents types and defaults] (which IDAES is NOT using).

Given that the bulk of IDAES docstrings have settled on google-style doc strings, we should probably look into making a google-style formatter that we use in IDAES.

@lbianchi-lbl
Copy link
Contributor Author

This specific issue (i.e. the backward-incompatible change) has been addressed by Pyomo/pyomo#2853, so this can be closed. #1196 is to discuss more long-term solutions related to the general autogenerated docstring functionality.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backward-compat Affects backward compatibility bug Something isn't working documentation Documentations comments and requests Priority:High High Priority Issue or PR
Projects
No open projects
Status: Done
Development

No branches or pull requests

4 participants