TST: Add Notebook tests #203
Conversation
Tests are skipped by default. Run `pytest -m "slow"` in order
to run slow tests.
Also require upgrading to pytest ~=6.0.0 in order to read
pyproject.toml configs.
Change pyproject.toml black table to use extend-exclude, which
also excludes anything gitignored
|
Not sure if we want to run notebook tests in CI, but if so, it's as simply as adding |
|
I think it's a good idea to run all the notebook tests in CI too. |
|
My only comment is to add some comments to test_notebook since some of the lines are somewhat opaque. Best, |
Codecov Report
@@ Coverage Diff @@
## master #203 +/- ##
=======================================
Coverage 93.48% 93.48%
=======================================
Files 34 34
Lines 3425 3425
=======================================
Hits 3202 3202
Misses 223 223 Continue to review full report at Codecov.
|
|
Added notebook tests to CI - I'll check the CI report, but if it works, should we remove papermill? |
|
Looks like you need to add sympy as a requirement, fix a timebase issue, etc. to get these tests working. |
|
Wdym timebase issue? It looks like the four failures were 1 sympy, 2 dead kernels, and 1 ZeroDivisionError (solved in my other PR, but I can move that commit over to a separate PR if you'd like since it's independent.) |
|
Given what you said about the fix, maybe it is better to merge this in after merging in the other pull request? |
|
Yeah, definitely post #185. At that point it would be a question of whether we can resolve the DeadKernelError in GH actions, send them to the AMATH server for test, or just skip them. |
Add jupytext dev requirements. Jupytext creates notebooks from scripts. Add publish_notebook.py script to regenerate and run notebook. Demonstrated on the shortest notebook we have. Added some flake8 config to handle the idiosyncracies of python files created from notebooks (long comments and imports not at top)
|
Added method to refactor and test notebooks much faster by putting data generation in My thoughts were that we needed:
I think this solution, demonstrated on Goal is to get all examples in .py format, then use @akaptano , @znicolaou , what are your thoughts? |
|
Having trouble with the pre-commit hook for jupytext: mwouts/jupytext#974 |
|
Ok @akaptano , @znicolaou, this PR should be good to review - in particular, look at the new notebook directory format. I removed the CI testing for notebooks not yet converted to this new format because they were running for several hours, which really gets in the way of development. As notebooks get converted to the new format, the test will automatically pick them up. A good example of the power of this commit: testing notebook 5 went from 30+ minutes to 7.5 seconds! 🚀 |
|
@znicolaou and @Jacob-Stevens-Haas Let's try to get this done this week, and get the release published. I am worried about delaying the release too long, considering the substantial changes that have been made in the past weeks. Thanks for your monumental efforts on the code! Best, |
|
Reminder (to myself) that when a new release comes out we need to update all the binder links in the notebooks. Presumably there is some automatic way to do this but it is easy to do, just hard to remember to do! |
…ests TST: Add Notebook tests
This broke with a refactoring a while ago and needed adjustment to new xarray versions. Reported in neuralhydrology/neuralhydrology#133. Also fixes an issue where the metrics csv file would be empty for multi-frequency runs.
To test this change, run
and
The first shows that regular testing ignores the notebooks. The second shows that the tests can be selected.