bird-house / flyingpigeon

WPS processes for climate model data, indices and extreme events
http://flyingpigeon.readthedocs.io/en/latest/
Apache License 2.0
19 stars 15 forks source link

Notebooks in documentation #319

Closed nilshempelmann closed 4 years ago

nilshempelmann commented 4 years ago

Overview

added notebooks added environment-docs.yml

This PR now fixes #318

Zeitsperre commented 4 years ago

@nilshempelmann The recent commits have fixed the build. See: https://readthedocs.org/projects/flyingpigeon/builds/10654824/.

I leave the rest of this to you unless you want another set of eyes to look at it.

nilshempelmann commented 4 years ago

So, not a problem with their anaconda build? The latest Add sphinx_rtd_theme for local building failed again

nilshempelmann commented 4 years ago

@Zeitsperre : exclude sphinx_rtd_theme in environment-doc made it running. You might ask the readthedoc team for the reason.

huard commented 4 years ago

@nilshempelmann Updated the subset notebook. Note that using progress=True will create failures for testing, because we don't know yet how to tell nbval to wait for a process to finish.

nilshempelmann commented 4 years ago

@huard Great. I need to understand how to design test with notebooks. Planed for the next days. could this be an option:

while out.getStatus() != 'ProcessSucceeded':
    time.sleep(1)
huard commented 4 years ago

Could be ! I don't think anyone really invested time in solving this yet. Also, I suggest you experiment with get(asobj), it's more user-friendly.

tlvu commented 4 years ago

I need to understand how to design test with notebooks

The core is leveraging the nbval (https://github.com/computationalmodelling/nbval) py.test plugin.

It allows to assert the notebook output did not change (exception thrown will also alter the output and caught). Hence the output need to be sanitized (mechanism provided by nbval) to remove any randomness (ex: timestamp change).

Since it's run py.test style, there is no user inputs, so notebooks should somehow still well behave even if user input is missing.

Your while out.getStatus() != 'ProcessSucceeded': is a great suggestion. But should add a max timeout to avoid waiting forever in the case the process is stuck in a queue with "max number of connections reached" kind of error.

See this PR https://github.com/bird-house/finch/pull/112 which added a make target test-notebooks and this PR https://github.com/bird-house/finch/pull/118 which added the refresh-notebooks make target.

We also have a full Jenkins test-suite that combines notebooks from various repos (ex: Pavics-sdi, Finch, Raven) and daily test those notebooks against our PAVICS production deployment or any other PAVICS deployment https://github.com/Ouranosinc/PAVICS-e2e-workflow-tests.

nilshempelmann commented 4 years ago

@tlvu Thanks for the links! I ll do my homework and check it out. https://github.com/bird-house/flyingpigeon/issues/320

tlvu commented 4 years ago

@tlvu Thanks for the links! I ll do my homework and check it out.

320

Remember a while ago, in one of a birdhouse meeting, I mentioned giving a lightning talk at a Canarie conference about automating Jenkins deployment, that's what's running the test suite above.

Consequently, if you want to deploy a Jenkins completely pre-configured to run the exact test-suite above, it's super duper easy to get rolling, see instructions here https://github.com/Ouranosinc/jenkins-config.