-
Notifications
You must be signed in to change notification settings - Fork 90
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
Format and lint notebooks #817
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
❗ Your organization needs to install the Codecov GitHub app to enable full functionality. Additional details and impacted files@@ Coverage Diff @@
## main #817 +/- ##
=======================================
Coverage ? 73.88%
=======================================
Files ? 31
Lines ? 2003
Branches ? 0
=======================================
Hits ? 1480
Misses ? 523
Partials ? 0 ☔ View full report in Codecov by Sentry. |
|
Hey @itcarroll for future reference, I don't log in to @MattF-NSIDC often so I don't see the notifications sent there :) Did the failing check not also update $ pre-commit run -a uv-lock
uv-lock..................................................................Failed
- hook id: uv-lock
- files were modified by this hook
warning: `VIRTUAL_ENV=/home/robatt/.cache/pre-commit/repo16nnfz_r/py_env-python3.12` does not match the project environment path `.venv` and will be ignored
Using Python 3.12.5 interpreter at: /home/robatt/.local/share/miniforge3/envs/earthaccess/bin/python3
Resolved 222 packages in 1.12s
Added blinker v1.8.2
Added flask v3.0.3
Added itsdangerous v2.2.0
Added werkzeug v3.0.4
$ git status
On branch main
Your branch is up to date with 'origin/main'.
Changes not staged for commit:
(use "git add <file>..." to update what will be committed)
(use "git restore <file>..." to discard changes in working directory)
modified: pyproject.toml
modified: uv.lock |
Oh, and thanks so much for taking on this important piece of technical debt!! 🙇 |
Understand now that the uv-lock hook is throwing an error, so that's why it's not updating the The error I could not make any sense of was
I tried reinstalling everything. What eventually made the error go away was updating the |
That's great! Thanks so much for tracking down a fix. That's a really weird error message, I don't know what to make of it 😆 I have some family in town so I can't give a full review until perhaps in the middle of the coming week! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
pre-commit.ci autofix |
6ece411
to
ea752f8
Compare
Closes #788
Re-enable the format and lint tool for notebook (.ipynb) files everywhere in the repo (which turns out to be in
notebooks
,docs/howto
, and mostlydocs/tutorials
).The only lint rules I found to be not great for notebooks are T201 and T203, which are prohibitions on
print
andpprint
in production code. I ignored them for "*.ipynb" in[tool.ruff.lint.per-file-ignores]
.I found two packages missing from
[project.optional-dependencies.docs]
that are needed for the notebooks to run, and added them witha minimum dependency taken from the version pip would install.uv add --extra docs dask cftime
.I did not attempt to fix some remaining errors in the notebooks that are beyond the scope of linting. They did not all run when I started and still don't all run (but not because of missing packages!).
Pull Request (PR) draft checklist - click to expand
contributing documentation
before getting started.
title such as "Add testing details to the contributor section of the README".
example
closes #1
. SeeGitHub docs - Linking a pull request to an issue.
CHANGELOG.md
with details about your change in a section titled## Unreleased
. If such a section does not exist, please create one. FollowCommon Changelog for your additions.
README.md
with details of changes to theearthaccess interface, if any. Consider new environment variables, function names,
decorators, etc.
Click the "Ready for review" button at the bottom of the "Conversation" tab in GitHub
once these requirements are fulfilled. Don't worry if you see any test failures in
GitHub at this point!
Pull Request (PR) merge checklist - click to expand
Please do your best to complete these requirements! If you need help with any of these
requirements, you can ping the
@nsidc/earthaccess-support
team in a comment and wewill help you out!
Request containing "pre-commit.ci autofix" to automate this.
📚 Documentation preview 📚: https://earthaccess--817.org.readthedocs.build/en/817/