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 empty autodoc processes list silent docs build failure #96

Merged

Conversation

tlvu
Copy link

@tlvu tlvu commented Jul 20, 2020

Currently, autodoc failure are treated as warnings only, resulting in
empty processes list, see
https://pavics-sdi.readthedocs.io/projects/raven/en/latest/processes.html.

This fix will turn these warnings into build failure on RtD and on
Travis-CI so we can catch those errors earlier.

Right now these warnings are falling silently !

Also changed Travis-CI config so it also builds Epub and Latex format as RtD to catch RtD errors before PR is merged.

Common fixes for those warnings are backported here.

Bonus we also catch nonexisting document reference and missing reference
in toctree, see below sample Raven warnings during docs build.

Raven succesful build logs with warnings on RtD:
https://readthedocs.org/api/v2/build/11487109.txt

/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/index.rst:14:
WARNING: toctree contains reference to nonexisting document 'pyraven'
WARNING: autodoc: failed to import process 'processes.RavenProcess' from
module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.RavenGR4JCemaNeigeProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.RavenMOHYSEProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process 'processes.RavenHMETSProcess'
from module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process 'processes.RavenHBVECProcess'
from module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.ObjectiveFunctionProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
looking for now-outdated files... none found
pickling environment... done
checking consistency...
/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/notebooks/Region_selection.ipynb:
WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/notebooks/Running_models_with_multiple_timeseries_files.ipynb:
WARNING: document isn't included in any toctree

This PR has been tested on Raven PR Ouranosinc/raven#293, Finch PR bird-house/finch#130, FlyingPigeon bird-house/flyingpigeon#337, bird-house/flyingpigeon#336, Emu bird-house/emu#105

Currently, autodoc failure are treated as warnings only, resulting in
empty processes list, see
https://pavics-sdi.readthedocs.io/projects/raven/en/latest/processes.html.

This fix will turn these warnings into build failure on RtD and on
Travis-CI so we can catch those errors earlier.

Right now they are falling silently !

Bonus we also catch nonexisting document reference and missing reference
in toctree, see below sample Raven warnings during docs build.

Raven succesful build logs with warnings on RtD:
https://readthedocs.org/api/v2/build/11487109.txt

/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/index.rst:14:
WARNING: toctree contains reference to nonexisting document 'pyraven'
WARNING: autodoc: failed to import process 'processes.RavenProcess' from
module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.RavenGR4JCemaNeigeProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.RavenMOHYSEProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process 'processes.RavenHMETSProcess'
from module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process 'processes.RavenHBVECProcess'
from module 'raven'; the following exception was raised:
No module named 'xclim.core'
WARNING: autodoc: failed to import process
'processes.ObjectiveFunctionProcess' from module 'raven'; the following
exception was raised:
No module named 'xclim.core'
looking for now-outdated files... none found
pickling environment... done
checking consistency...
/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/notebooks/Region_selection.ipynb:
WARNING: document isn't included in any toctree
/home/docs/checkouts/readthedocs.org/user_builds/pavics-raven/checkouts/latest/docs/source/notebooks/Running_models_with_multiple_timeseries_files.ipynb:
WARNING: document isn't included in any toctree
@cehbrecht cehbrecht added enhancement New feature or request documentation labels Jul 20, 2020
@cehbrecht cehbrecht added this to the 1.0.0 milestone Jul 20, 2020
@cehbrecht cehbrecht added this to In progress in Niamey (Summer 2020) via automation Jul 20, 2020
@cehbrecht
Copy link
Member

Not sure if we would like to have this optional (fail on warning). It might be hard to get the api docs rendered on RTD ...

@tlvu
Copy link
Author

tlvu commented Jul 20, 2020

Not sure if we would like to have this optional (fail on warning). It might be hard to get the api docs rendered on RTD ...

@cehbrecht Could you elaborate on the problem? I'll put a comment next to that option so downstream project can choose to disable it. But I'd rather have it turn on by default to avoid silent failure unless you tell me it's a very frequent problem and this option will just make our life much harder than the advantage it brings.

@cehbrecht
Copy link
Member

Not sure if we would like to have this optional (fail on warning). It might be hard to get the api docs rendered on RTD ...

@cehbrecht Could you elaborate on the problem? I'll put a comment next to that option so downstream project can choose to disable it. But I'd rather have it turn on by default to avoid silent failure unless you tell me it's a very frequent problem and this option will just make our life much harder than the advantage it brings.

@tlvu I'm always happy when build on RTD works ... it is sometimes troublesome due to the dependencies. Feel free to merge ... we can adapt in the projects.

@tlvu
Copy link
Author

tlvu commented Jul 20, 2020

I'm always happy when build on RTD works ... it is sometimes troublesome due to the dependencies. Feel free to merge ... we can adapt in the projects.

Thanks @cehbrecht. I'll merge this PR once I deploy this change to Raven and confirm it works. Maybe I'll hit the problem you mentioned and will reconsider. Will for sure add a comment to the RtD option so other projects feel comfortable disabling it if they need.

@cehbrecht
Copy link
Member

I'm always happy when build on RTD works ... it is sometimes troublesome due to the dependencies. Feel free to merge ... we can adapt in the projects.

Thanks @cehbrecht. I'll merge this PR once I deploy this change to Raven and confirm it works. Maybe I'll hit the problem you mentioned and will reconsider. Will for sure add a comment to the RtD option so other projects feel comfortable disabling it if they need.

@tlvu ok. Thanks :)

@tlvu
Copy link
Author

tlvu commented Jul 24, 2020

@cehbrecht @Zeitsperre
Update: I've tested this PR on Raven (PR Ouranosinc/raven#293) and removing all the RtD build warnings was not as simple as I expected but I got it finally (relevant changes backported here).

To further reduce RtD build to fail after PR is merged, I've also modified Travis-CI to also build Epub and Latex format (to do the same thing as RtD) so Travis-CI can also catch RtD errors.

tlvu added a commit to Ouranosinc/pywps that referenced this pull request Jul 27, 2020
Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (`sphinx-build -W`).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

See PR bird-house/cookiecutter-birdhouse#96 with
the silent doc build error.
tlvu added a commit to Ouranosinc/pywps that referenced this pull request Jul 27, 2020
Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (`sphinx-build -W`).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

See PR bird-house/cookiecutter-birdhouse#96 with
the silent doc build error.
tlvu added a commit to Ouranosinc/pywps that referenced this pull request Jul 27, 2020
Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (`sphinx-build -W`).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

See PR bird-house/cookiecutter-birdhouse#96 with
the silent doc build error.
…compatible with latest pytest 6.0.0

See computationalmodelling/nbval#139

Error from Finch Travis-CI build https://travis-ci.org/github/bird-house/finch/jobs/713031486

$ make test-notebooks
Copying notebook output sanitizer ...
Running notebook-based tests
============================= test session starts ==============================
platform linux -- Python 3.7.8, pytest-6.0.0, py-1.9.0, pluggy-0.13.1 -- /home/travis/miniconda/envs/finch/bin/python
cachedir: .pytest_cache
rootdir: /home/travis/build/bird-house/finch, configfile: setup.cfg
plugins: nbval-0.9.5, notebook-0.6.0, flake8-1.0.6
collected 0 items / 1 error

==================================== ERRORS ====================================
________________________ ERROR collecting test session _________________________
Traceback (most recent call last):
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/hooks.py", line 286, in __call__
    return self._hookexec(self, self.get_hookimpls(), kwargs)
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/manager.py", line 93, in _hookexec
    return self._inner_hookexec(hook, methods, kwargs)
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/manager.py", line 87, in <lambda>
    firstresult=hook.spec.opts.get("firstresult") if hook.spec else False,
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/callers.py", line 208, in _multicall
    return outcome.get_result()
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/callers.py", line 80, in get_result
    raise ex[1].with_traceback(ex[2])
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/pluggy/callers.py", line 187, in _multicall
    res = hook_impl.function(*args)
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/nbval/plugin.py", line 115, in pytest_collect_file
    return IPyNbFile(path, parent)
  File "/home/travis/miniconda/envs/finch/lib/python3.7/site-packages/_pytest/nodes.py", line 95, in __call__
    warnings.warn(NODE_USE_FROM_PARENT.format(name=self.__name__), stacklevel=2)
pytest.PytestDeprecationWarning: Direct construction of IPyNbFile has been deprecated, please use IPyNbFile.from_parent.
See https://docs.pytest.org/en/stable/deprecations.html#node-construction-changed-to-node-from-parent for more details.
=========================== short test summary info ============================
ERROR  - pytest.PytestDeprecationWarning: Direct construction of IPyNbFile ha...
!!!!!!!!!!!!!!!!!!!! Interrupted: 1 error during collection !!!!!!!!!!!!!!!!!!!!
=============================== 1 error in 0.14s ===============================
Makefile:128: recipe for target 'test-notebooks' failed
make: *** [test-notebooks] Error 2
tlvu added a commit to bird-house/finch that referenced this pull request Jul 29, 2020
…oved-doc-build

Refresh cookiecutter for more robust doc build.

This PR applies the cookiecutter PR bird-house/cookiecutter-birdhouse#96 to ensure no silent ReadTheDocs build failure and for Travis-CI to also catch ReadTheDocs build failure before PR is merged.

Successful RtD generation: https://pavics-sdi.readthedocs.io/projects/finch/en/test-rtd-build/ and matching RtD build logs: https://readthedocs.org/api/v2/build/11555597.txt (commit f5c857e) 

Changes:

* Turn RtD warnings to build failure so they do not fail silently.

* Change Travis-CI build config to also build Epub and Latex doc format to catch RtD failure on Travis-CI before PR is merged.

* Various changes to remove all warnings in doc build since warnings will now fail the doc build.

* setup.py: fix `make develop` not installing `requirements_dev.txt` as expected

* add notebooks `finch-usage.ipynb` and `dap_subset.ipynb` to RtD

Unrelated changes part of this PR (sorry !):

* Other cookiecutter changes that are unapplied previously are also part of this PR.  You might want to look at each commit to avoid the noises.
@tlvu tlvu merged commit caedce8 into master Jul 29, 2020
Niamey (Summer 2020) automation moved this from In progress to Done Jul 29, 2020
@tlvu tlvu deleted the fix-silent-docs-build-failure-empty-autodoc-processes-list branch July 29, 2020 20:20
tlvu added a commit to Ouranosinc/raven that referenced this pull request Jul 29, 2020
…in-autodoc-directive

Fix RtD silent build failure in autodoc directive.

This PR fixes RtD silent build failure in autodoc directive.  The process list https://pavics-sdi.readthedocs.io/projects/raven/en/latest/processes.html is currently empty and it's not supposed to be like that.  We never got alerted because it's failing silently.

Fixed RtD build: https://pavics-sdi.readthedocs.io/projects/raven/en/test-rtd-build/processes.html and matching RtD build log https://readthedocs.org/api/v2/build/11555558.txt (commit baba22f)

Changes:

* Turn RtD warnings to build failure so they do not fail silently.

* Change Travis-CI build config to also build Epub and Latex doc format to catch RtD failure on Travis-CI before PR is merged.

* Various changes to remove all warnings in doc build since warnings will now fail the doc build.

Unrelated changes part of this PR (sorry !):

* Refresh cookiecutter (first commit in this PR is technically not part of the intended fix of this PR).

* Remove Travis-CI directive to only build `master` branch so all dev branch gets build on Travis-CI.  Otherwise we either have to create a PR when branch is not even ready or hardcode branch name in `.travis.yml` and have to remember to remove it at the end, very annoying.  See comment in #292 (comment)

## Additional Information

Matching cookiecutter PR bird-house/cookiecutter-birdhouse#96

@richardarsenault You are probably not interested in the low level details in this PR.  I just want to draw your attention to the fact that I found 2 notebooks missing in the toctree (Region_selection.ipynb, Running_models_with_multiple_timeseries_files.ipynb).  I added them to the end of the toctree, wonder if you are okay with the ordering or these 2 notebooks were intended to not appear in the doc.
tlvu added a commit to geopython/pywps that referenced this pull request Jul 30, 2020
ext_autodoc: support RST anonymous link

# Overview

Sphinx "Duplicate explicit target name" warning is problematic when
warning are turned into errors (`sphinx-build -W`).

And we would want warnings to turn into error because autodoc import
failure are treated as warnings only, resulting into silent
documentation build failure if those warnings do not fail the build.

Not adding directly to Metadata class but subclassing Metadata because Metadata is being used to generate the xml response and it can also be serialized to and deserialized from json format. So bigger impact and not required for what I needed to solve.

# Related Issue / Discussion

See PR bird-house/cookiecutter-birdhouse#96 with the silent doc build error.

See commit bird-house/flyingpigeon@c97ebca and bird-house/emu@cfcaedf for actual real usage of this change.
tlvu added a commit to bird-house/emu that referenced this pull request Jul 30, 2020
…ust-doc-build

Refresh cookiecutter for more robust doc build.

This PR applies the cookiecutter PR bird-house/cookiecutter-birdhouse#96 to ensure no silent ReadTheDocs build failure and for Travis-CI to also catch ReadTheDocs build failure before PR is merged.

Successful RtD generation: https://emu.readthedocs.io/en/test-rtd-build/ and matching RtD build logs: https://readthedocs.org/api/v2/build/11555381.txt

Changes:

* Turn RtD warnings to build failure so they do not fail silently.

* Change Travis-CI build config to also build Epub and Latex doc format to catch RtD failure on Travis-CI before PR is merged.

* Various changes to remove all warnings in doc build since warnings will now fail the doc build.

  * For doc build only, to work-around warnings, use a new PYWPS release to support RST anonymous links, currently installing PYWPS from source (with commit hash locked so it is reproducible) since no release yet (PR geopython/pywps#542)

  * Anaconda build badge do not exist anymore so it had to be removed.

Unrelated changes part of this PR (sorry !):

* Other cookiecutter changes that are unapplied previously are also part of this PR.  You might want to look at each commit to avoid the noises.
Zeitsperre pushed a commit to Ouranosinc/raven that referenced this pull request Aug 17, 2023
…in-autodoc-directive

Fix RtD silent build failure in autodoc directive.

This PR fixes RtD silent build failure in autodoc directive.  The process list https://pavics-sdi.readthedocs.io/projects/raven/en/latest/processes.html is currently empty and it's not supposed to be like that.  We never got alerted because it's failing silently.

Fixed RtD build: https://pavics-sdi.readthedocs.io/projects/raven/en/test-rtd-build/processes.html and matching RtD build log https://readthedocs.org/api/v2/build/11555558.txt (commit 5521c48)

Changes:

* Turn RtD warnings to build failure so they do not fail silently.

* Change Travis-CI build config to also build Epub and Latex doc format to catch RtD failure on Travis-CI before PR is merged.

* Various changes to remove all warnings in doc build since warnings will now fail the doc build.

Unrelated changes part of this PR (sorry !):

* Refresh cookiecutter (first commit in this PR is technically not part of the intended fix of this PR).

* Remove Travis-CI directive to only build `master` branch so all dev branch gets build on Travis-CI.  Otherwise we either have to create a PR when branch is not even ready or hardcode branch name in `.travis.yml` and have to remember to remove it at the end, very annoying.  See comment in #292 (comment)

## Additional Information

Matching cookiecutter PR bird-house/cookiecutter-birdhouse#96

@richardarsenault You are probably not interested in the low level details in this PR.  I just want to draw your attention to the fact that I found 2 notebooks missing in the toctree (Region_selection.ipynb, Running_models_with_multiple_timeseries_files.ipynb).  I added them to the end of the toctree, wonder if you are okay with the ordering or these 2 notebooks were intended to not appear in the doc.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation enhancement New feature or request
Projects
No open projects
Development

Successfully merging this pull request may close these issues.

None yet

3 participants