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

Feature/notebook tests #108

Merged
merged 23 commits into from
Jul 20, 2022
Merged

Conversation

michalk8
Copy link
Collaborator

@michalk8 michalk8 commented Jul 13, 2022

In this PR:

  • add pre-commit to format notebooks (using black)
  • fix some notebooks not being runnable (usually related to the online option change)
  • fix Python code lexer in notebooks
  • add regression tests for 7 notebooks (including a new CI job)
  • add bibtex entries in the documentation instead of raw links
  • add autogenerated Colab link instead of hardcoded ones

TODOs:

  • fix blackening of notebooks (problem with --run-all-files)
  • limit time execution
  • exclude long running notebooks, e.g., docs/notebooks/OTT_&_POT.ipynb?
  • udpate CONTRIBUTING.rst

closes #104

@michalk8 michalk8 self-assigned this Jul 13, 2022
@review-notebook-app
Copy link

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@codecov-commenter
Copy link

codecov-commenter commented Jul 13, 2022

Codecov Report

Merging #108 (def1358) into main (f405c21) will decrease coverage by 7.86%.
The diff coverage is 40.95%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #108      +/-   ##
==========================================
- Coverage   87.27%   79.40%   -7.87%     
==========================================
  Files          41       42       +1     
  Lines        3708     3904     +196     
  Branches      431      446      +15     
==========================================
- Hits         3236     3100     -136     
- Misses        364      677     +313     
- Partials      108      127      +19     
Impacted Files Coverage Δ
ott/core/discrete_barycenter.py 15.58% <ø> (-70.13%) ⬇️
ott/core/gromov_wasserstein.py 85.71% <ø> (-12.61%) ⬇️
ott/core/implicit_differentiation.py 97.26% <ø> (-2.74%) ⬇️
ott/core/momentum.py 91.66% <ø> (ø)
ott/core/quad_problems.py 85.48% <ø> (-3.23%) ⬇️
ott/core/sinkhorn.py 94.04% <ø> (-0.80%) ⬇️
ott/core/sinkhorn_lr.py 91.70% <ø> (-0.88%) ⬇️
ott/core/unbalanced_functions.py 100.00% <ø> (ø)
ott/geometry/geometry.py 78.27% <ø> (-7.38%) ⬇️
ott/geometry/grid.py 90.26% <ø> (-0.89%) ⬇️
... and 35 more

@michalk8 michalk8 force-pushed the feature/notebook-tests branch from facc7f5 to 8dca3ff Compare July 15, 2022 09:49
@michalk8 michalk8 added enhancement New feature or request documentation Improvements or additions to documentation labels Jul 18, 2022
@michalk8 michalk8 marked this pull request as ready for review July 18, 2022 14:08
@michalk8 michalk8 requested a review from marcocuturi July 19, 2022 09:47
Copy link
Contributor

@marcocuturi marcocuturi left a comment

Choose a reason for hiding this comment

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

Thanks so much Michal for all of this work, it's fantastic to see these improvements in the doc!

@marcocuturi marcocuturi merged commit 540b74c into ott-jax:main Jul 20, 2022
michalk8 added a commit that referenced this pull request Jun 27, 2024
* Add pre-commit to black format of notebook

* Add notebook testing on CI

* Use `black-nb`, fix lexer, add black config

* Format notebooks again

* Rerun Hessians notebook with correct precond

* Use `--nbval-lax`

* Autogenerate collab badge

* Add `!pip install ...` cell for notebooks

* Update notebook bibtex references

* Update bibliography

* Add rest of the bibtex entries

* [ci skip] Update pre-commit

* Switch to `testbook`, add notebook tests

* Switch linting CI to Python3.9

* Update CONTRIBUTING.md

* Try different kernel name, lint back on 3.8

* Install ipykernel on the CI

* Remove grid notebook test

* Fix wrong package links

* Fix linter

* Update core.rst

* Update core.rst

Co-authored-by: marcocuturi <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add CI for notebooks
3 participants