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

CI testing of the notebooks #84

Open
sacdallago opened this issue Oct 29, 2020 · 3 comments
Open

CI testing of the notebooks #84

sacdallago opened this issue Oct 29, 2020 · 3 comments
Assignees
Labels
enhancement New feature or request prio:low

Comments

@sacdallago
Copy link
Owner

sacdallago commented Oct 29, 2020

#81 and #83 highlight something discussed a while ago, which is CI testing of notebooks and examples.

The strategy up until now has been to manually test notebooks and examples prior to a release, but this did not always work out as smoothly as I had hoped.

We might want to start running a CI job testing notebooks and examples when a PR is merged to develop on GitLab. For both cases, the expected outcome should be error-free execution.

I see two issues with this as it stands now:

  1. the goPredSim examples expect prior download of files (all other examples can be run directly from the repo)
  2. Many notebooks require download of files, and we would have to ignore the Colab init on local testing
@sacdallago sacdallago added enhancement New feature or request prio:low labels Oct 29, 2020
@sacdallago sacdallago added this to the Version v0.1.5 milestone Oct 29, 2020
@konstin
Copy link
Collaborator

konstin commented Nov 8, 2020

Preliminary results: The code below runs all notebooks and reports which pass and which fail. However, it will run all cells, including the pip install, as papermill does not have a way to skip cells (nteract/papermill#429). So we need a way to skip cells, even if it's just "skip the first cell unconditionally". We could use the nbconvert cli which can do that with some tricks (nteract/papermill#494 (comment)) or we can try wrapping that so that the pip installs are skipped during test.

The good thing about this is that downloading files works same as in colab.

import sys
from pathlib import Path

import papermill
import pytest


@pytest.mark.parametrize(
    "notebook_name", sorted(i.stem for i in Path("notebooks").glob("*.ipynb"))
)
def test_notebook(notebook_name, tmp_path):
    papermill.execute_notebook(
        Path("notebooks").joinpath(notebook_name).with_suffix(".ipynb"),
        tmp_path.joinpath("output.ipynb"),
        stdout_file=sys.stdout,
        stderr_file=sys.stderr,
    )

@alex-treebeard
Copy link

Hey @konstin -- just been dropping by on notebook testing issues as i've release nbmake.

Only issue is this is the first case i've seen where cell-skipping is required. Potentially an interesting feature. Keen to hear your feedback, if any!

@sacdallago
Copy link
Owner Author

Should we bump this up or close for now?

I did relize that some notebooks need updating in the mantime, e.g. the ones from the protocol & supervised extract. Probably anotherone to add to the #125 thread

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request prio:low
Projects
None yet
Development

No branches or pull requests

4 participants