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

Notebook linting #625

Open
pnkraemer opened this issue Feb 8, 2022 · 1 comment
Open

Notebook linting #625

pnkraemer opened this issue Feb 8, 2022 · 1 comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation testing and CI Unit tests, coverage and continuous integration

Comments

@pnkraemer
Copy link
Collaborator

Is your feature request related to a problem? Please describe.
ProbNum's source is linted rather extensively, but the notebooks are not at all.
This leads to some discrepancies in code quality, which are not great, especially since it affects the tutorials.

Describe the solution you'd like.
As discussed offline, we could apply black, isort, and pylint to the notebooks via nbqa https://github.com/nbQA-dev/nbQA as part of the CI. This would detect some basic issues with the code quality in jupyter notebooks before merging them into main.

Additional context

  • As also discussed offline, it might not be a bad idea to skip the pre-commit hook (at least for the beginning), because when committing changes to a notebook while having the notebook opened, the file contents get overwritten and jupyter runs into some issues.
  • Implementing this could involve a PR with a large diff, because almost all notebooks would presumably be affected.
  • It could make sense to implement this in 2 steps: black and isort could be done straightforwardly, pylint will require a large number of --disable="..." statements and/or nontrivial refactoring in the notebooks.
@pnkraemer pnkraemer added documentation Improvements or additions to documentation testing and CI Unit tests, coverage and continuous integration dependencies Pull requests that update a dependency file labels Feb 8, 2022
@marvinpfoertner
Copy link
Collaborator

marvinpfoertner commented Feb 11, 2022

black supports notebook formatting out-of-the-box now: https://github.com/psf/black/releases/tag/21.8b0.
We simply need to add black[jupyter] in format-requirements.txt and everything should work without needing to modify anything else :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
dependencies Pull requests that update a dependency file documentation Improvements or additions to documentation testing and CI Unit tests, coverage and continuous integration
Projects
None yet
Development

No branches or pull requests

2 participants