Skip to content

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

Merged
dylwil3 merged 6 commits intoastral-sh:mainfrom
dylwil3:newlines-notebook
Jan 9, 2025
Merged

[pycodestyle] Handle each cell separately for too-many-newlines-at-end-of-file (W391)#15308
dylwil3 merged 6 commits intoastral-sh:mainfrom
dylwil3:newlines-notebook

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).

@github-actions
Copy link
Contributor

github-actions bot commented Jan 6, 2025 •
edited
Loading

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
MichaReiser reviewed Jan 7, 2025
View reviewed changes
@MichaReiser
Copy link
Member

MichaReiser commented Jan 7, 2025 •
edited
Loading

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
MichaReiser
MichaReiser approved these changes Jan 8, 2025
View reviewed changes
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
@BrewTestBot BrewTestBot mentioned this pull request Jan 10, 2025
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@MichaReiser MichaReiser MichaReiser approved these changes

Assignees

No one assigned

Labels

notebook Related to (Jupyter) notebooks preview Related to preview mode features rule Implementing or modifying a lint rule

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

W391 reported but not fixed in Jupyter notebook

2 participants

@dylwil3 @MichaReiser