search

bandframework / Taweret

Python package for Bayesian Model Mixing
https://bandframework.github.io/Taweret/
MIT License
8 stars 8 forks source link

merge branch 100_jupyterbook into main #110

Closed asemposki closed 3 months ago

asemposki commented 3 months ago

This PR will merge 100_jupyterbook into main, to include the Jupyter Book in the main branch of the repository. It will also allow for testing of the Jupyter Book and PR---before merging, I will push an error in a notebook and test if GitHub Actions catches this error. I will then push the fix of the error and comment that this merge is ready to occur. After this comment, @jared321 will review the PR.

codecov[bot] commented 3 months ago

Codecov Report

Attention: Patch coverage is 27.27273% with 8 lines in your changes missing coverage. Please review.

Project coverage is 66.60%. Comparing base (f48739f) to head (49e9fbd). Report is 129 commits behind head on main.

Files with missing lines Patch % Lines
src/Taweret/mix/bivariate_linear.py 0.00% 8 Missing :warning:
Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #110 +/- ## ========================================== - Coverage 66.85% 66.60% -0.26% ========================================== Files 14 14 Lines 1554 1560 +6 ========================================== Hits 1039 1039 - Misses 515 521 +6 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

asemposki commented 3 months ago

I see Codecov wants coverage of the try/except statements I wrote to handle the multiprocessing runtime issues we had. I can attempt to write some pytests for that.

asemposki commented 3 months ago

The Jupyter Book failed to build when I pushed a bug in one of the notebooks that prevented the import of one of Taweret's modules. The Book ran every notebook and then failed at the end as it could not finish executing the Bivariate_Test.ipynb notebook. This would typically be treated as a warning but with our warnings as errors setting, this failed and the Book did not deploy. I will re-instate that module now and push the fix.

asemposki commented 3 months ago

Excellent, the bug fix worked and the Jupyter Book is again building.

@jared321, anytime you want to review this PR, go ahead.

asemposki commented 3 months ago

OK, I've reviewed everything here, and everything looks good from a cursory glance, except the couple of changes mentioned in the comments above, which I'll check this afternoon.

asemposki commented 3 months ago

I checked the notebooks, and I see that Dan's is still showing all verbose sampling, which is not necessary and only wastes time printing and clutters the interface for a reader in the Jupyter Book, so I will clean that up as well before we merge this in. I should be able to do that quickly now.

jared321 commented 3 months ago

@asemposki Have we now attended to all issues raised during the review? If so, then I will try to break/fix the Jupyter book as you did to confirm that the new PR-based action is failing as intended and review all our changes one last time.

asemposki commented 3 months ago

I checked the RTD and everything looks good right now. I am good with you breaking and fixing the notebooks now. :)

jared321 commented 3 months ago

@asemposki

Looks good from my side.

asemposki commented 3 months ago

OK great! I will merge this and then make a branch from main to work on the content of the Book a bit more before the end of day tomorrow. I will also update the SDK to v0.4, since we'll need that done too.