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

Upstream changes #103

Closed
wants to merge 292 commits into from
Closed

Upstream changes #103

wants to merge 292 commits into from

Conversation

Zeitsperre
Copy link
Collaborator

Overview

This PR fixes #99

Changes:

  • Integrated changes from upstream master, including re-enabling tox
  • Restored removed documentation

Related Issue / Discussion

Re-enabling tox is particularly high in the priorities list, given the changes that have come to pass with Travis CI. At the very least, we now know the risks associated with tying ourselves to a specific service API.

There are quite a lot of changes in here. Even though I performed a rebase-type merge to gradually solve the merge conflicts (time-consuming), there is likely a lot here to unpack. Some of my goals were as follows:

  • Make pytest optional (selecting against it removes much of the pytest-based code we have).
  • Make Click optional to benefit from upstream changes that use argparse instead.
  • Leverage tox where possible and have CI services rely more on makefile recipes.
  • Restore most of the documentation that was previously removed so users/developers know how to contribute here.

Additional Information

#99
Ouranosinc/xclim#509

kpfleming and others added 30 commits October 28, 2018 11:57
ReadTheDocs is no longer a GitHub Service, it's now a GitHub App, so it automatically generates a webhook for itself.
English course link was pointing to the same place as the Spanish one.
Use maintained bump2version instead of abandoned bumpversion
Remove unnecessary ReadTheDocs instruction
Use pytest instead of py.test per upstream recommendation, #dropthedot
…ting

Update formatting of CONTRIBUTING.rst
tlvu and others added 17 commits January 5, 2021 13:07
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
So Travis-CI can catch ReadTheDocs build failure before PR is merged.

ReadTheDocs also build epub and latex format in addition to html.

See Raven commit
Ouranosinc/raven@7b535b4
Partially revert 61b4176.

numpy existed in Raven because we install xclim in Raven.

See FlyingPigeon comnit
bird-house/flyingpigeon@2aa72b2
pip install makes it easier to grab any random missing dependencies from PyPI and in general is the promoted/accepted way to install modern python packages. It's also in the standard library and almost every python installation (unless you do a hyper minimal conda install). It's just better in general.
…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
The newest major release of pytest (6.0.0) is now compatible with nbval at and above 0.9.6. No more need for specific pinned versions.
# Conflicts:
#	.travis.yml
#	CONTRIBUTING.rst
#	LICENSE
#	README.rst
#	cookiecutter.json
#	docs/conf.py
#	docs/console_script_setup.rst
#	docs/prompts.rst
#	docs/pypi_release_checklist.rst
#	docs/travis_pypi_setup.rst
#	docs/tutorial.rst
#	hooks/post_gen_project.py
#	requirements_dev.txt
#	setup.cfg
#	setup.py
#	tests/test_bake_project.py
#	tox.ini
#	{{cookiecutter.project_repo_name}}/.gitignore
#	{{cookiecutter.project_repo_name}}/.travis.yml
#	{{cookiecutter.project_repo_name}}/CONTRIBUTING.rst
#	{{cookiecutter.project_repo_name}}/Makefile
#	{{cookiecutter.project_repo_name}}/README.rst
#	{{cookiecutter.project_repo_name}}/docs/installation.rst
#	{{cookiecutter.project_repo_name}}/requirements_dev.txt
#	{{cookiecutter.project_repo_name}}/setup.cfg
#	{{cookiecutter.project_repo_name}}/setup.py
#	{{cookiecutter.project_repo_name}}/tox.ini
#	{{cookiecutter.project_repo_name}}/{{cookiecutter.project_slug}}/__init__.py
#	{{cookiecutter.project_repo_name}}/{{cookiecutter.project_slug}}/cli.py
#	{{cookiecutter.project_repo_name}}/{{cookiecutter.project_slug}}/tests/test_{{cookiecutter.project_slug}}.py
@Zeitsperre Zeitsperre added the enhancement New feature or request label Jan 5, 2021
@Zeitsperre Zeitsperre self-assigned this Jan 5, 2021
@tlvu
Copy link

tlvu commented Jan 7, 2021

This is a large PR. Will be a few days before I can have time to digest it. Hope it's okay.

@Zeitsperre Zeitsperre marked this pull request as ready for review January 7, 2021 15:46
@Zeitsperre
Copy link
Collaborator Author

@tlvu It still isn't working, so no rush. The merge process took a few hours and still needs a cleanup.

@Zeitsperre
Copy link
Collaborator Author

Closed. Was able to get this working with #110

@Zeitsperre Zeitsperre closed this May 5, 2021
@Zeitsperre Zeitsperre deleted the upstream-changes branch May 5, 2021 17:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Implement tox for easier automated processing