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

Adds script to run notebooks and catch any errors #2616

Merged
merged 10 commits into from
Nov 28, 2022
Merged

Conversation

aamster
Copy link
Contributor

@aamster aamster commented Nov 18, 2022

#2601

  • Adds script to run notebooks and catch any errors
  • This script is run as part of a new github actions workflow which is triggered when the target branch is "master". Example run: https://github.com/AllenInstitute/AllenSDK/actions/runs/3498866966/jobs/5859760665. Many notebooks are failing. Some won't run until we update data on S3. Created Fix issues in notebooks #2611 to fix the others.
  • The notebooks parameterize the output_dir to manage files output by the notebook using a tag named "parameters". This is in the papermill (library used to run notebooks) docs. Before, these paths referenced internal paths or fake paths, and so the notebook wouldn't run.
  • Comments out blocks of notebook that output excessive data (~1 TB) or take an excessive amount of time to run (~1 hr)
  • Skips behavior_ophys_session notebook since it can only be run internally. Didn't want to add a 2.5 GB NWB file to the repo or change the notebook to use the cloud api.

The notebooks parameterize the output_dir to manage files output by the notebook
Moves behavior_ophys_session notebook to internal since it can only be run internally

Adds github actions to run the notebook runner
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

jobs:
run_notebooks:
name: Notebook runner
runs-on: "ubuntu-latest"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What version of python would this use?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The default version of python that ships with the "ubuntu-latest" image, which currently is 3.8, but looks like it will be 3.10 on december 1:
https://github.com/actions/runner-images/blob/main/images/linux/Ubuntu2004-Readme.md

Copy link
Contributor

@mikejhuang mikejhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It was interesting to see how notebooks can be parameterized and automated with papermill.

@aamster
Copy link
Contributor Author

aamster commented Nov 23, 2022

@mikejhuang thanks for feedback. I think I addressed all your feedback.

Copy link
Contributor

@mikejhuang mikejhuang left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good!
What if the output of the notebook changes without throwing errors? Is there a way to review it?

@aamster
Copy link
Contributor Author

aamster commented Nov 28, 2022

What if the output of the notebook changes without throwing errors? Is there a way to review it?

@mikejhuang the notebook changes are automatically committed, like in this commit . But we can review the auto commit.

@aamster aamster merged commit 4b45ab6 into rc/2.16.1 Nov 28, 2022
@aamster aamster mentioned this pull request Nov 28, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants