-
-
Notifications
You must be signed in to change notification settings - Fork 18
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: Drop testing notebooks; only run them #205
ENH: Drop testing notebooks; only run them #205
Conversation
✔️ Deploy Preview for carpentries-dmri ready! 🔨 Explore the source changes: b7dbaa9 🔍 Inspect the deploy log: https://app.netlify.com/sites/carpentries-dmri/deploys/623397609d724f0008acf624 😎 Browse the preview: https://deploy-preview-205--carpentries-dmri.netlify.app |
Drop testing notebooks; only run them. `pytest` and the `nbval` plugin were being used to actually collect notebooks for testing, and to test cells that output some value. However, `nbval`'s `timeout` flag to explicitly set the cell execution time is not working properly, and most `preprocessing` notebook cells were not being run. Thus, this commit favors running all cells over testing the few cells that print some output value.
@kaitj @josephmje The CIs are doing their job, and the preprocessing notebook seems to crash: Any chance to have a look and fix that, please? Thanks. A separate PR would be best, as I'd need to slightly clean the additions in here once we get all notebooks running. |
Took a quick look but I suspect it has something to do with the way the command is called.
It looks like its adding the newline characters with how the notebook cells are run in the CI now. As far as I know, nothing has changed since we ran those tests to time these cells (and when we had tried it with |
Thanks for the investigation Jason. Maybe defining the paths/filenames as variables will likely help to avoid having an endless, unreadable line, and hopefully make it digestible to the CI (?) Splitting the |
I've just come across this: Might be worthwhile trying: looks to accept/successfully apply the |
Drop testing notebooks; only run them.
pytest
and thenbval
pluginwere being used to actually collect notebooks for testing, and to test
cells that output some value. However,
nbval
'stimeout
flag toexplicitly set the cell execution time is not working properly, and most
preprocessing
notebook cells were not being run.Thus, this commit favors running all cells over testing the few cells
that print some output value.