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

Run Jupyter notebooks in CI #322

Merged
merged 8 commits into from
Apr 30, 2024
Merged

Conversation

JostMigenda
Copy link
Member

Closes #252 by running all (ccsn & presn) model notebooks and some general notebooks as part of the integration tests.

(For now, I’m exempting several general notebooks, which e.g. require a hard-coded path to the SNOwGLoBES or models directory. In some cases, there may be workarounds; but it’s gonna get more straightforward with some of the changes planned for v2.0.)

As a nice side effect of testing this, I discovered a few issues with existing notebooks which I’ve also fixed. (In particular, the notebooks I copied over from the snewpy-models-ccsn repo in #313 were not the latest versions and missed changes from #282, so I’m restoring those here.)

@JostMigenda JostMigenda added this to the v1.5 milestone Apr 29, 2024
@JostMigenda JostMigenda force-pushed the JostMigenda/ModelNotebookTests branch from 5511294 to d9e0994 Compare April 30, 2024 00:18
Test output warns “this will become a hard error in future nbformat versions”
Copy link
Collaborator

@mcolomermolla mcolomermolla left a comment

Choose a reason for hiding this comment

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

I see no issue in merging this.
Side notes:

  • Maybe we can avoid the saving of pdfs figures directly on the notebook?
  • For Fornax_2022: I like the way BH progenitors are handled now and that one can now pass directly a times array to get the initial spectra (good this point was solved) :)

@JostMigenda
Copy link
Member Author

Thanks @mcolomermolla! I’ve added a commit to avoid writing those files out by default; good point.

@JostMigenda JostMigenda merged commit a2f4d36 into main Apr 30, 2024
10 checks passed
@JostMigenda JostMigenda deleted the JostMigenda/ModelNotebookTests branch April 30, 2024 13:26
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.

check that model notebooks and tutorials run using unit tests
2 participants