Lint Jupyter notebooks: put *.ipynb in ruff's include - #412
Merged
Conversation
The feedstock PR DOI-USGS#14 upload landed (pyprms >=0.10.0 replaces packaging <26.3), so the last packaging <26.3 pin anywhere is gone. Move the item from Open to Done.
jmccreight
force-pushed
the
lint_notebooks
branch
from
August 25, 2026 21:21
943e719 to
a247aba
Compare
[tool.ruff] include covered only *.py, so `ruff check .` and CI never
linted notebooks. But .github/scripts/ruff_check.sh greps for .ipynb and
passes those paths explicitly, which overrides include, so the
pre-commit hook did lint them. Notebook problems were invisible until
someone edited a notebook and got a wall of errors they had not caused.
Notebooks now go in include, with two rules exempted:
E501 - they are narrative; wrapping cells to 79 columns hurts
readability and churns diffs for no correctness gain.
I001 - several notebooks import a module purely for its side effect
(hvplot.xarray registers a .hvplot accessor) and must do so
after the module it extends. Sorting moves those imports above
their dependency, which was verified against 06.
For the same line-length reason notebooks are linted but not
auto-formatted: [tool.ruff.format] excludes them, and ruff_check.sh
passes --force-exclude so the hook honors that despite naming paths
explicitly.
This surfaced three broken cells that no test covers, all in notebooks
CI does not run:
- a stray ")" in prms_5.2.1_performance.ipynb
- a string opened with " and closed with ' in
sagehen-postprocess-maps.ipynb
- pl.Path used with no `import pathlib` in
plot_4_performance_domains.ipynb
Four malformed # noqa directives are also repaired, including a "# noaq"
typo. Ruff rejects "# noqa, after xr" as malformed and silently ignores
it, so the unused-import fixer would have deleted the very hvplot
imports those comments existed to protect -- which would have broken
06_flow_graph_starfit.ipynb, one of the notebooks CI runs. Per review,
the explanatory comments now sit above the import and the directives are
spelled `# noqa: F401`.
The remaining 32 unused imports were audited individually rather than
blind-fixed; none are side-effect imports. Every notebook referencing
hvplot still imports it. The notebooks were not executed as part of this
change.
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jmccreight
force-pushed
the
lint_notebooks
branch
from
August 25, 2026 21:22
a247aba to
66fa246
Compare
A stray ) in prms_5.2.1_performance.ipynb was a syntax error that masked the rest of its cell. With it gone: a semicolon-joined statement, unused imports and assignments, and bare except clauses narrowed to except AssertionError. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
jmccreight
changed the title
*.ipynb in ruff's include
jmccreight
added a commit
to jmccreight/pywatershed
that referenced
this pull request
Aug 26, 2026
…-ups whats-new.rst: replace the :pull:`XXX` placeholders with 414. CLAUDE.md: always carry the PR template's checklist into a PR body, even when nothing in it applies, rather than dropping the section. MAINTENANCE.md: park PR DOI-USGS#412's two deferred follow-ups -- the pre-commit ruff-check hook's types: [python] may skip notebook-only commits, and pyproject's optional extra lacks the holoviews floor the environment files carry.
Merged
6 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Notebooks were never linted.
[tool.ruff] includelisted only*.py, soruff check .— and therefore CI — skipped every.ipynb. The pre-commit hookpassed staged notebook paths explicitly, which overrides
include, so it didlint them. Notebook problems were invisible until someone edited a notebook and
was met with errors they had not caused.
This adds
**/*.ipynbtoinclude, with two rules exempted for notebooks:E501— wrapping narrative cells to 79 columns hurts readability and churnsdiffs for no correctness gain.
I001— several notebooks import a module purely for its side effect(
hvplot.xarrayregisters a.hvplotaccessor) and must do so after themodule it extends; sorting moves those imports above their dependency.
Notebooks are linted but not auto-formatted: the formatter wraps to
line-length regardless of the
E501exemption, which would reflow every cellin every notebook.
Turning the lint on surfaced four broken cells that no test covered — two stray
), a string opened with"and closed with', and a use ofpl.Pathwithno
import pathlib. Three notebooks needed further cleanup once the syntaxerrors stopped masking it: unused imports, unused assignments, a
semicolon-joined statement, and bare
exceptclauses narrowed toexcept AssertionError. It also repairs four malformed# noqadirectives(including a
# noaqtypo) that ruff silently ignored — which meant theunused-import fixer would have deleted the
hvplotimports those directiveswere meant to protect.
.github/scripts/ruff_check.shgains--force-excludeso the pre-commit hookhonors
[tool.ruff.format] excludewhen paths are passed explicitly. TheMAINTENANCE.mdchange is unrelated bookkeeping carried on this branch: theconda-forge feedstock item moved to Done.
Closes #xxxxTests addedPerformance benchmarks addedPerformance regression benchmarks runwhats-new.rstNew functions/methods are listed inapi.rstor it's sub rsts?Docs
Look for this PR number in our read-the-docs builds where you can browse on-line.
You can alternatively get a CI-build of the docs by entering the PR number for the
###in https://github.com/DOI-USGS/pywatershed/pull/###/checks then clicking onDocumentation Buildand finally looking for thedocumentation-htmlartifact which will download as a zip file.