-
Notifications
You must be signed in to change notification settings - Fork 43
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 building fixes #1744
Doc building fixes #1744
Conversation
ready for testing on OSX |
ready for testing on Win |
ready for testing on Linux |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems to be failing on all jenkins builds (Win, OSX, Linux) in
=== Build HTML Docs from ReST Files ===
section with error:
subprocess.CalledProcessError: Command '['sphinx-build', '-v', '-b', 'html', '-d', '/Users/sasview/Jenkins/workspace/SasView-OSX10.13-ghprb/sasview/docs/sphinx-docs/build/doctrees', '-W', '--keep-going', '/Users/sasview/Jenkins/workspace/SasView-OSX10.13-ghprb/sasview/docs/sphinx-docs/source-temp', '/Users/sasview/Jenkins/workspace/SasView-OSX10.13-ghprb/sasview/docs/sphinx-docs/build/html']' returned non-zero exit status 1.
@wpotrzebowski, yes that is expected. The docs don't build cleanly yet -- this is just one step closer to getting them to build at all. Your test with jenkins highlights another problem, however. The jenkins builds are using much older versions of sphinx than CI (both Travis and GitHub Actions); it's not practical to support such a wide range of sphinx versions in one project. I suspect that we're now reliant on newer features of sphinx and that's breaking the build. Looking in I suspect the path forward is to fix the jenkins environment to better match the development environment. |
@llimeht Indeed different yaml files in |
travis and github actions are picking up the current version of sphinx: 3.4.3
|
I have no idea what conda is showing there but it looks pretty wrong. Sphinx 1.8.5 is from March 2019 and there have been 31 releases of Sphinx since then, right through to 3.4.3 in Jan 2021 (changelog). |
I think that was from this page https://anaconda.org/anaconda/sphinx. Am guessing that is just listing which versions are available as conda packages? A bit worrisome however how frequent the releases are coming? |
Interesting with versioning. I've managed to get sphinx 3.4.3 to OSX pull request builder. Let see if this prevents build from failing |
ready for testing on OSX |
Failing with sphinx 3.4.3 but with different error: |
I suspect that's from the old version of matplotlib that is pinned there (2.1 or 2.2 rather than 3.3 that travis/github actions is using). |
I've dropped off the very last commit that was causing issues with ancient sphinx so that the rest of these should be safe to merge. Note that errors in building the docs in github actions are still ignored at present and that output documentation is most likely not quite as intended. Fixing the docs so that they build correctly and making CI gate on that is still a rather important task. For reference, most of the error are of the form:
and these come from monkeypatching in |
sas.models is a monkeypatched sasmodels that breaks sphinx autodoc. Fixing this in a way that is compatible with both sphinx 1.8 and 3.x seems difficult, so put a sphinx version check on the fix. See SasView#1744
ready for testing on OSX |
The major reason for the current failure to build the sasview docs is sasmodels being monkeypatched into
sas.models
. Having loaded all the classes, sphinx knows thatsas.models
should exist but then can't actually load any source for them. That results in the rather odd errors that we have been seeing:Note that this PR is not enough to actually make the documentation build cleanly, with the following 19 warnings/errors still emitted:
Given that there are several PRs that touch those files already waiting to be merged, touching those files now will just create merge conflicts and so these problems need to be fixed later.
(There's also a handful of other misc fixes that I spotted while chasing this failure over the past few days.)