terrapower / armi

An open-source nuclear reactor analysis automation framework that helps design teams increase efficiency and quality
https://terrapower.github.io/armi/
Apache License 2.0
224 stars 87 forks source link

Activate sphinx-needs extension in doc config. #1641

Closed ntouran closed 3 months ago

ntouran commented 8 months ago

The docs have lots of sphinx needs directives in them, including implementation (.. impl::) and tests (.. test::). These are defined by the sphinx-needs extension, which is in our requirements but not actually activated in our docs config. This activates it in the docs. The net result is that hundreds of errors in the doc build process are resolved.


Checklist

john-science commented 8 months ago

I explicitly removed this.

ARMI does not contain any "requirements", and thus the sphinx-needs tooling fails.

As ARMI can't move the requirements into this repo, it will never be possible to build a set of requirements into the ARMI docs. And this is by design.

I would like to close this PR.

ntouran commented 8 months ago

and thus the sphinx-needs tooling fails.

Right now there are hundreds of errors when building the docs due to impl and test directives not found. So this is kind of a problem in either direction. We should at least suppress or otherwise handle the errors (perhaps by including 'dummy'/no-op definitions of these directives in our conf.py with detailed docs about why they're in there. Then we could also remove sphinx-needs from project requirements.

However, ...

ARMI can't move the requirements into this repo,

I don't think this has to be 100% true. I believe we can come up with a way to bring at least some framework-specific requirements into the repo. as a best practice and good example. That said, it'd certainly be useful to come up with a plan that enables people to bring their own proprietary downstream QA info to the framework cleanly. I haven't thought much about this but it's worth pondering.

john-science commented 8 months ago

As long as the docs still build, I'm sure this is fine.

I thought the docs didn't build with this dependency added. But maybe it was the conf.py links to sphinx-needs that were the problem.

john-science commented 8 months ago

This does not seem to change the error messages being thrown by the ARMI HTML docs build.

For instance, both log files have this error:

/home/runner/work/armi/armi/armi/__init__.py:docstring of armi:1: ERROR: Unknown directive type "impl".

.. impl:: Settings are used to define an ARMI run.
    :id: I_ARMI_SETTING1
    :implements: R_ARMI_SETTING

And there are ~260 such "ERROR"s. None of them affect the build in either case.

For reference: