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

ENH: Prefer nbmake plugin for testing notebooks #207

Closed
wants to merge 1 commit into from
Closed

ENH: Prefer nbmake plugin for testing notebooks #207

wants to merge 1 commit into from

Conversation

jhlegarreta
Copy link
Contributor

Prefer using the nbmake plugin over nbval for testing notebooks.

nbval's timeout flag to explicitly set the cell execution time is
not working properly, and most preprocessing notebook cells were not
being run.

Documentation:
https://pypi.org/project/nbmake/

@netlify
Copy link

netlify bot commented Apr 2, 2022

Deploy Preview for carpentries-dmri ready!

Name Link
🔨 Latest commit 9c29392
🔍 Latest deploy log https://app.netlify.com/sites/carpentries-dmri/deploys/6248e906285b610009f3e3e5
😎 Deploy Preview https://deploy-preview-207--carpentries-dmri.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site settings.

@jhlegarreta
Copy link
Contributor Author

Does not seem to provide a cell-wise report, but at least the timeout parameter is being correctly processed (although its 1h value seems to fall short for Python 3.9 and topup):
https://github.com/carpentries-incubator/SDC-BIDS-dMRI/runs/5800703673?check_suite_focus=true#step:13:58

Prefer using the `nbmake` plugin over `nbval` for testing notebooks.

`nbval`'s `timeout` flag to explicitly set the cell execution time is
not working properly, and most `preprocessing` notebook cells were not
being run.

Documentation:
https://pypi.org/project/nbmake/
@alex-treebeard
Copy link

hi @jhlegarreta -- I maintain nbmake (sorry to intrude)

Does not seem to provide a cell-wise report

Why would this help? Feature requests welcome 👍

@jhlegarreta
Copy link
Contributor Author

jhlegarreta commented Apr 4, 2022

hi @jhlegarreta -- I maintain nbmake (sorry to intrude)

@alex-treebeard No worries; we appreciate the time spent developing the plugin, and your interest on our use case.

Does not seem to provide a cell-wise report

Why would this help? Feature requests welcome +1

It's maybe a matter of preferences/nice-to-have feature, although, in practice, when a cell fails we still know which cell is the culprit.

A cell-wise report, as we had in the past using pytest and nbval, allows to have a clean report/summary, e.g.
https://github.com/carpentries-incubator/SDC-BIDS-dMRI/runs/5572292930?check_suite_focus=true#step:13:28

Also, without diving into the error trace, the report/summary shows the error type. In addition, pytest and nbval run all cells (and any subsequent notebook) even if one fails earlier, which nbmake does not seem to do at the moment (or at least I did not find the documentation to do so).

The latter is useful because if we make changes to multiple cells across different notebooks, we'd rather know the errors at once to save CI wait times, be able to fix all issues at once, and ultimately save ourselves time. Even more so when testing (including our setup steps) takes so long.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status:unfinished at workbench Unfinished. Closed by the Workbench transition tool.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants