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

[pycodestyle] Handle each cell separately for too-many-newlines-at-end-of-file (W391) #15308

Merged
merged 6 commits into from
Jan 9, 2025

Conversation

dylwil3
Copy link
Collaborator

@dylwil3 dylwil3 commented Jan 6, 2025

Jupyter notebooks are converted into source files by joining with newlines, which confuses the check too-many-newlines-at-end-of-file (W391). This PR introduces logic to apply the check cell-wise (and, in particular, correctly handles empty cells.)

Closes #13763

@dylwil3 dylwil3 added rule Implementing or modifying a lint rule preview Related to preview mode features notebook Related to (Jupyter) notebooks labels Jan 6, 2025
@dylwil3
Copy link
Collaborator Author

dylwil3 commented Jan 6, 2025

One aesthetic question is whether internal cells should still be allowed to have a single trailing newline (at the moment this is what the fix produces/allows).

Copy link
Contributor

github-actions bot commented Jan 6, 2025

ruff-ecosystem results

Linter (stable)

✅ ecosystem check detected no linter changes.

Linter (preview)

ℹ️ ecosystem check detected linter changes. (+2 -17 violations, +0 -0 fixes in 3 projects; 52 projects unchanged)

PlasmaPy/PlasmaPy (+0 -1 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

- docs/notebooks/dispersion/stix_dispersion.ipynb:cell 34:1:1: W391 [*] Extra newline at end of file

bokeh/bokeh (+2 -5 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview --select ALL

+ examples/output/jupyter/push_notebook/Basic Usage.ipynb:cell 14:1:1: W391 [*] Extra newline at end of cell
- examples/output/jupyter/push_notebook/Basic Usage.ipynb:cell 14:1:1: W391 [*] Too many newlines at end of file
- examples/output/jupyter/push_notebook/Continuous Updating.ipynb:cell 8:1:1: W391 [*] Extra newline at end of file
- examples/output/jupyter/push_notebook/Jupyter Interactors.ipynb:cell 8:1:1: W391 [*] Extra newline at end of file
- examples/output/jupyter/push_notebook/Numba Image Example.ipynb:cell 28:1:1: W391 [*] Extra newline at end of file
+ examples/server/api/notebook_embed.ipynb:cell 7:1:1: W391 [*] Extra newline at end of cell
- examples/server/api/notebook_embed.ipynb:cell 7:1:1: W391 [*] Too many newlines at end of file

mlflow/mlflow (+0 -11 violations, +0 -0 fixes)

ruff check --no-cache --exit-zero --ignore RUF9 --no-fix --output-format concise --preview

- docs/source/llms/llm-evaluate/notebooks/question-answering-evaluation.ipynb:cell 29:1:1: W391 [*] Extra newline at end of file
- docs/source/llms/llm-evaluate/notebooks/rag-evaluation.ipynb:cell 19:1:1: W391 [*] Extra newline at end of file
- docs/source/llms/rag/notebooks/retriever-evaluation-tutorial.ipynb:cell 50:1:1: W391 [*] Extra newline at end of file
- docs/source/traditional-ml/hyperparameter-tuning-with-child-runs/notebooks/parent-child-runs.ipynb:cell 15:1:1: W391 [*] Extra newline at end of file
- docs/source/traditional-ml/serving-multiple-models-with-pyfunc/notebooks/MME_Tutorial.ipynb:cell 25:1:1: W391 [*] Extra newline at end of file
- examples/evaluation/qa-evaluation.ipynb:cell 29:1:1: W391 [*] Extra newline at end of file
- examples/evaluation/rag-evaluation.ipynb:cell 16:1:1: W391 [*] Extra newline at end of file
- examples/h2o/random_forest.ipynb:cell 6:1:1: W391 [*] Extra newline at end of file
- examples/llms/RAG/retriever-evaluation-tutorial.ipynb:cell 48:1:1: W391 [*] Extra newline at end of file
- examples/rapids/mlflow_project/notebooks/rapids_mlflow.ipynb:cell 14:1:1: W391 [*] Extra newline at end of file
- examples/sklearn_elasticnet_wine/train.ipynb:cell 6:1:1: W391 [*] Extra newline at end of file

Changes by rule (1 rules affected)

code total + violation - violation + fix - fix
W391 19 2 17 0 0

@MichaReiser
Copy link
Member

MichaReiser commented Jan 7, 2025

One aesthetic question is whether internal cells should still be allowed to have a single trailing newline (at the moment this is what the fix produces/allows).

I think we should and doing so would be consistent with the formatter.

It would be very annoying if you write a cell with a newline at the end, hit save and Ruff fixes away the newline before you had a chance to write the next content.

@dylwil3 dylwil3 requested a review from MichaReiser January 7, 2025 17:37
Copy link
Member

@MichaReiser MichaReiser left a comment

Choose a reason for hiding this comment

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

Thanks

@dylwil3 dylwil3 merged commit b0905c4 into astral-sh:main Jan 9, 2025
21 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
notebook Related to (Jupyter) notebooks preview Related to preview mode features rule Implementing or modifying a lint rule
Projects
None yet
Development

Successfully merging this pull request may close these issues.

W391 reported but not fixed in Jupyter notebook
2 participants