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 example notebooks CI #791

Merged
merged 24 commits into from
Jun 28, 2024
Merged

Run example notebooks CI #791

merged 24 commits into from
Jun 28, 2024

Conversation

juanitorduz
Copy link
Collaborator

@juanitorduz juanitorduz commented Jun 28, 2024

I am revisiring #624 . We can do it via readthedocs but I want to get feedback on this simpler solution.


📚 Documentation preview 📚: https://pymc-marketing--791.org.readthedocs.build/en/791/

Copy link

codecov bot commented Jun 28, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.86%. Comparing base (a933b70) to head (4cc1922).
Report is 183 commits behind head on main.

Additional details and impacted files
@@           Coverage Diff           @@
##             main     #791   +/-   ##
=======================================
  Coverage   93.86%   93.86%           
=======================================
  Files          28       28           
  Lines        2950     2950           
=======================================
  Hits         2769     2769           
  Misses        181      181           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@juanitorduz juanitorduz added the docs Improvements or additions to documentation label Jun 28, 2024
scripts/run_notebooks/runner.py Show resolved Hide resolved
scripts/run_notebooks/runner.py Outdated Show resolved Hide resolved
@juanitorduz juanitorduz marked this pull request as ready for review June 28, 2024 11:13
@juanitorduz
Copy link
Collaborator Author

@wd60622 @carlosagostini We have had many API changes, and the notebooks are hard to keep up 🙈 (see, for example, #793, which I discovered when working on this). I suggest this solution that is flexible and easy to manage. It runs locally very fast and in the CI takes 10 min, which is comparable to the tests we have in place. WDYT?

@juanitorduz juanitorduz requested a review from wd60622 June 28, 2024 11:15
@juanitorduz juanitorduz added enhancement New feature or request tests labels Jun 28, 2024
Copy link
Contributor

@wd60622 wd60622 left a comment

Choose a reason for hiding this comment

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

Looks great and will be a lifesafer.

How about a command in the Makefile as well? make notebooks or something like that to do the runner

@juanitorduz
Copy link
Collaborator Author

Looks great and will be a lifesafer.

How about a command in the Makefile as well? make notebooks or something like that to do the runner

Good idea! Added in fe3cdf7

@wd60622
Copy link
Contributor

wd60622 commented Jun 28, 2024

Looks great and will be a lifesafer.
How about a command in the Makefile as well? make notebooks or something like that to do the runner

Good idea! Added in fe3cdf7

Can you run that twice in a row? i.e. does the runner still work if the files already exist?

@juanitorduz
Copy link
Collaborator Author

@wd60622 im sorry but I don't understand the question 🙈😄

@wd60622
Copy link
Contributor

wd60622 commented Jun 28, 2024

@wd60622 im sorry but I don't understand the question 🙈😄

does

make run_notebooks
make run_notebooks

work?
Sometimes things don't work if the files already exist

@juanitorduz
Copy link
Collaborator Author

@wd60622 im sorry but I don't understand the question 🙈😄

does

make run_notebooks
make run_notebooks

work? Sometimes things don't work if the files already exist

Woks locally ✅

@juanitorduz juanitorduz requested a review from wd60622 June 28, 2024 12:08
@juanitorduz juanitorduz merged commit 576cf45 into main Jun 28, 2024
12 checks passed
@juanitorduz juanitorduz deleted the ci_nb_script branch June 28, 2024 12:26
twiecki pushed a commit that referenced this pull request Sep 10, 2024
* run notebooks init

* change kernel name

* change kernel name

* change kernel name

* rm docs

* add nb

* fix nb

* fig dpi

* update model object

* no output

* test mmm notebooks

* fix path

* graphviz

* sudo

* paralelize and Path

* reqs

* undo

* comments

* try budget allocation

* add quickstart

* ignore budget allocation

* add make command

* use make
twiecki pushed a commit that referenced this pull request Sep 10, 2024
* run notebooks init

* change kernel name

* change kernel name

* change kernel name

* rm docs

* add nb

* fix nb

* fig dpi

* update model object

* no output

* test mmm notebooks

* fix path

* graphviz

* sudo

* paralelize and Path

* reqs

* undo

* comments

* try budget allocation

* add quickstart

* ignore budget allocation

* add make command

* use make
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs Improvements or additions to documentation enhancement New feature or request maintenance request discussion tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants