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

Investigate use of testbook for testing Jupyter notebooks #925

Open
matthewfeickert opened this issue Jul 7, 2020 · 5 comments
Open

Investigate use of testbook for testing Jupyter notebooks #925

matthewfeickert opened this issue Jul 7, 2020 · 5 comments
Labels
good first issue Good for newcomers research experimental stuff SciPy Conference Resulted from discussions at a SciPy conference tests pytest

Comments

@matthewfeickert
Copy link
Member

Description

At SciPy 2020 @rohitsanj gave a (very good) lightning talk on nteract's testbook which is used for unit testing Jupyter Notebooks. Given that Carol Willing mentioned it is designed to be complimentary with papermill it might be worth seeing if it can be used in our testing of the example notebooks in CI.

cc @phinate as neos uses lots of notebooks.

@matthewfeickert matthewfeickert added good first issue Good for newcomers research experimental stuff tests pytest SciPy Conference Resulted from discussions at a SciPy conference labels Jul 7, 2020
@rohitsanj
Copy link

Hi @matthewfeickert, thanks for the mention! We would be more than excited to see testbook be used by neos or any other repo.

If you'd like, I could try writing some of the tests using testbook for any notebook that you can point out - win win situation 😄

Thanks again!

cc @MSeal

@matthewfeickert
Copy link
Member Author

If you'd like, I could try writing some of the tests using testbook for any notebook that you can point out

@rohitsanj fantastic! Thanks for the incredibly generous offer.

I'll first describe how we use papermill in our CI to get your thoughts on where testbook might be the best fit. All of our examples in our documentation are Jupyter notebooks rendered with Sphinx. We use papermill to run all of these notebooks in our CI each PR to ensure that we never break our examples. As some of our examples involve quite a bit of computation this can be slow. Maybe it would make more sense to try and unit test all the functions defined in the notebooks in CI to speed things up and then just run the full papermill tests on a nightly schedule job?

If you think this makes sense, maybe a good place to start is our simplest example notebook: hello-world.ipynb

@MSeal
Copy link

MSeal commented Jul 7, 2020

Btw for context, papermill is good for integration testing, testbook is good for unittesting. If you have notebooks with high branching factor or want to avoid expensive computation associated with functions with making dummy parameters, testbook should be a good choice. So I think you have the right idea of how testbook might fit in.

@matthewfeickert
Copy link
Member Author

matthewfeickert commented Jul 7, 2020

or want to avoid expensive computation associated with functions with making dummy parameters

Yeah, at the moment we're more just running the notebooks through to make sure the public APIs don't break and checking a few values with scrapbook, but the notebook CI takes about 17 minutes at the moment.

@MSeal
Copy link

MSeal commented Jul 8, 2020

At Netflix we reused our scheduler in a dedicated deployment to parallelize out all the papermill integration tests, which helped with integration test wall time substantially.

Unfortunately GitHub actions doesn't have higher core options yet: https://github.community/t/higher-cpu-count-virtual-environments-planned/16338 so you do get stuck with the mostly serial execution times.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
good first issue Good for newcomers research experimental stuff SciPy Conference Resulted from discussions at a SciPy conference tests pytest
Projects
None yet
Development

No branches or pull requests

3 participants