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

Stabilize support for Jupyter Notebooks #12878

Merged
merged 3 commits into from
Aug 14, 2024

Conversation

dhruvmanila
Copy link
Member

@dhruvmanila dhruvmanila commented Aug 14, 2024

Summary

This PR stabilizes the support for Jupyter Notebooks and adds them to Ruff's default inclusion set.

Closes: #12456
Closes: astral-sh/ruff-vscode#546

Test Plan

  • Make sure all tests pass, updated snapshots are valid
  • Ecosystem checks?
  • Run Ruff in a repository containing both Python files and Jupyter notebooks

@dhruvmanila dhruvmanila added breaking Breaking API change notebook Related to (Jupyter) notebooks labels Aug 14, 2024
@MichaReiser MichaReiser added this to the v0.6 milestone Aug 14, 2024
`ruff format /path/to/notebook.ipynb` will always format `notebook.ipynb`.
Alternatively, passing the notebook file(s) to `ruff` on the command-line will always lint or format
the notebook. For example, `ruff check /path/to/notebook.ipynb` will always lint `notebook.ipynb`.
Similarly, `ruff format /path/to/notebook.ipynb` will always format `notebook.ipynb`.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we won't need this section anymore. we can move this section to another place that applies for all file types not just .ipynb

@dhruvmanila dhruvmanila force-pushed the dhruv/include-notebooks-by-default branch from e0128be to 80fb8e5 Compare August 14, 2024 10:12
@dhruvmanila dhruvmanila marked this pull request as ready for review August 14, 2024 10:21
Comment on lines 344 to 345
To lint or format files with additional file extensions, use the [`extend-include`](settings.md#extend-include) setting.

=== "pyproject.toml"

```toml
[tool.ruff]
extend-include = ["*.ipynb"]
```

=== "ruff.toml"

```toml
extend-include = ["*.ipynb"]
```

You can also change the default selection using the [`include`](settings.md#include) setting.
Copy link
Member Author

Choose a reason for hiding this comment

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

Any suggestion on what can be use for this example? I'm worried that it might signal to the users that we support that file extension. Using *.ipynb was ok because we do support it. This is the reason I've removed this example.

Copy link
Member

Choose a reason for hiding this comment

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

extend-include has an example with .pyw files. Although I think the example isn't needed because we already link to extend-include which has an example.

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, I think not having an example here is fine.

@dhruvmanila
Copy link
Member Author

There are couple more references in the docs that require updating which I'm doing right now.

Copy link
Contributor

github-actions bot commented Aug 14, 2024

ruff-ecosystem results

Linter (stable)

ℹ️ ecosystem check detected linter changes. (+1109 -0 violations, +0 -0 fixes in 8 projects; 46 projects unchanged)

alteryx/featuretools (+5 -0 violations, +0 -0 fixes)

+ docs/source/guides/time_series.ipynb:cell 1:4:1: E402 Module level import not at top of cell
+ docs/source/guides/time_series.ipynb:cell 1:6:1: E402 Module level import not at top of cell
+ docs/source/guides/time_series.ipynb:cell 1:7:1: E402 Module level import not at top of cell
+ docs/source/guides/time_series.ipynb:cell 1:8:1: E402 Module level import not at top of cell
+ docs/source/resources/frequently_asked_questions.ipynb:cell 101:5:8: E721 Use `is` and `is not` for type comparisons, or `isinstance()` for isinstance checks

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

+ docs/notebooks/analysis/fit_functions.ipynb:cell 12:3:39: NPY002 Replace legacy `np.random.normal` call with `np.random.Generator`
+ docs/notebooks/analysis/nullpoint.ipynb:cell 11:10:1: T201 `print` found
+ docs/notebooks/analysis/nullpoint.ipynb:cell 11:9:1: T201 `print` found
+ docs/notebooks/analysis/nullpoint.ipynb:cell 14:10:1: T201 `print` found
+ docs/notebooks/analysis/nullpoint.ipynb:cell 14:11:1: T201 `print` found
+ docs/notebooks/analysis/nullpoint.ipynb:cell 9:1:5: D103 Missing docstring in public function
+ docs/notebooks/analysis/swept_langmuir/find_floating_potential.ipynb:cell 24:29:42: FBT003 Boolean positional value in function call
+ docs/notebooks/analysis/swept_langmuir/find_floating_potential.ipynb:cell 24:29:48: FBT003 Boolean positional value in function call
+ docs/notebooks/analysis/swept_langmuir/find_floating_potential.ipynb:cell 24:30:42: FBT003 Boolean positional value in function call
+ docs/notebooks/analysis/swept_langmuir/find_floating_potential.ipynb:cell 24:30:48: FBT003 Boolean positional value in function call
... 160 additional changes omitted for project

apache/airflow (+31 -0 violations, +0 -0 fixes)

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

+ dev/stats/explore_pr_candidates.ipynb:cell 1:1:1: F821 Undefined name `sys`
+ dev/stats/explore_pr_candidates.ipynb:cell 1:1:1: I002 [*] Missing required import: `from __future__ import annotations`
+ dev/stats/explore_pr_candidates.ipynb:cell 1:3:1: E402 Module level import not at top of cell
+ dev/stats/explore_pr_candidates.ipynb:cell 1:3:1: I001 [*] Import block is un-sorted or un-formatted
+ dev/stats/explore_pr_candidates.ipynb:cell 1:4:1: E402 Module level import not at top of cell
+ dev/stats/explore_pr_candidates.ipynb:cell 1:4:8: TID253 `pandas` is banned at the module level
+ dev/stats/explore_pr_candidates.ipynb:cell 1:5:1: E402 Module level import not at top of cell
+ dev/stats/explore_pr_candidates.ipynb:cell 1:5:41: F401 [*] `get_important_pr_candidates.PrStat` imported but unused
+ dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: PTH123 `open()` should be replaced by `Path.open()`
+ dev/stats/explore_pr_candidates.ipynb:cell 2:1:8: SIM115 Use context handler for opening files
... 21 additional changes omitted for project

apache/superset (+668 -0 violations, +0 -0 fixes)

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

+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 10:3:15: Q000 [*] Single quotes found but double quotes preferred
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 10:3:1: T201 `print` found
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 10:3:38: Q000 [*] Single quotes found but double quotes preferred
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 11:1:1: PD901 Avoid using the generic variable name `df` for DataFrames
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 11:1:32: Q000 [*] Single quotes found but double quotes preferred
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 11:1:53: Q000 [*] Single quotes found but double quotes preferred
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:100:3: Q000 [*] Single quotes found but double quotes preferred
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:101:3: Q000 [*] Single quotes found but double quotes preferred
... 482 additional changes omitted for rule Q000
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:10:14: W291 [*] Trailing whitespace
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:237:13: COM812 [*] Trailing comma missing
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:254:19: COM812 [*] Trailing comma missing
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:258:89: E501 Line too long (145 > 88)
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:261:3: T201 `print` found
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:30:89: E501 Line too long (121 > 88)
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:49:89: E501 Line too long (92 > 88)
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:50:3: ERA001 Found commented-out code
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:50:89: E501 Line too long (99 > 88)
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:72:89: E501 Line too long (94 > 88)
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:81:3: ERA001 Found commented-out code
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 13:82:11: W291 [*] Trailing whitespace
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 15:3:89: E501 Line too long (95 > 88)
... 39 additional changes omitted for rule E501
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 15:7:22: COM812 [*] Trailing comma missing
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:11:9: T201 `print` found
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:19:5: T201 `print` found
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:1:13: ANN001 Missing type annotation for function argument `country`
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:1:5: ANN201 Missing return type annotation for public function `get_gdf`
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:1:5: D103 Missing docstring in public function
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:30:69: W291 [*] Trailing whitespace
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:9:24: ANN001 Missing type annotation for function argument `countries`
+ superset-frontend/plugins/legacy-plugin-chart-country-map/scripts/Country Map GeoJSON Generator.ipynb:cell 16:9:35: ANN001 Missing type annotation for function argument `subplot_width`
... 638 additional changes omitted for project

bokeh/bokeh (+160 -0 violations, +0 -0 fixes)

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

+ examples/models/structure/ModelStructureExample.ipynb:cell 11:2:5: T201 `print` found
+ examples/models/structure/ModelStructureExample.ipynb:cell 13:1:1: B018 Found useless expression. Either assign it to a variable or remove it.
+ examples/models/structure/ModelStructureExample.ipynb:cell 13:2:17: Q000 [*] Single quotes found but double quotes preferred
+ examples/models/structure/ModelStructureExample.ipynb:cell 13:2:40: Q000 [*] Single quotes found but double quotes preferred
+ examples/models/structure/ModelStructureExample.ipynb:cell 13:2:59: Q000 [*] Single quotes found but double quotes preferred
+ examples/models/structure/ModelStructureExample.ipynb:cell 13:2:64: Q000 [*] Single quotes found but double quotes preferred
+ examples/models/structure/ModelStructureExample.ipynb:cell 14:1:15: Q000 [*] Single quotes found but double quotes preferred
+ examples/models/structure/ModelStructureExample.ipynb:cell 14:1:1: F821 Undefined name `pd`
+ examples/models/structure/ModelStructureExample.ipynb:cell 2:1:1: I001 [*] Import block is un-sorted or un-formatted
+ examples/models/structure/ModelStructureExample.ipynb:cell 2:1:8: F401 [*] `bokeh` imported but unused
... 150 additional changes omitted for project

langchain-ai/langchain (+31 -0 violations, +0 -0 fixes)

+ libs/cli/langchain_cli/integration_template/docs/chat.ipynb:cell 11:4:89: E501 Line too long (102 > 88)
+ libs/cli/langchain_cli/integration_template/docs/chat.ipynb:cell 12:1:1: T201 `print` found
+ libs/cli/langchain_cli/integration_template/docs/chat.ipynb:cell 14:7:89: E501 Line too long (97 > 88)
+ libs/cli/langchain_cli/integration_template/docs/chat.ipynb:cell 3:5:89: E501 Line too long (98 > 88)
+ libs/cli/langchain_cli/integration_template/docs/document_loaders.ipynb:cell 12:1:1: T201 `print` found
+ libs/cli/langchain_cli/integration_template/docs/document_loaders.ipynb:cell 3:4:89: E501 Line too long (94 > 88)
+ libs/cli/langchain_cli/integration_template/docs/llms.ipynb:cell 3:5:89: E501 Line too long (98 > 88)
+ libs/cli/langchain_cli/integration_template/docs/provider.ipynb:cell 2:1:1: I001 [*] Import block is un-sorted or un-formatted
+ libs/cli/langchain_cli/integration_template/docs/provider.ipynb:cell 2:1:29: F401 [*] `__module_name__.Chat__ModuleName__` imported but unused
+ libs/cli/langchain_cli/integration_template/docs/provider.ipynb:cell 2:2:29: F401 [*] `__module_name__.__ModuleName__LLM` imported but unused
... 21 additional changes omitted for project

milvus-io/pymilvus (+11 -0 violations, +0 -0 fixes)

+ examples/hello_milvus.ipynb:cell 14:13:9: T201 `print` found
+ examples/hello_milvus.ipynb:cell 14:14:1: T201 `print` found
+ examples/hello_milvus.ipynb:cell 16:5:1: T201 `print` found
+ examples/hello_milvus.ipynb:cell 16:6:1: T201 `print` found
+ examples/hello_milvus.ipynb:cell 18:7:9: T201 `print` found
+ examples/hello_milvus.ipynb:cell 18:8:1: T201 `print` found
... 5 additional changes omitted for rule T201
+ examples/hello_milvus.ipynb:cell 2:1:1: I001 [*] Import block is un-sorted or un-formatted
... 4 additional changes omitted for project

pandas-dev/pandas (+33 -0 violations, +0 -0 fixes)

+ doc/source/user_guide/style.ipynb:cell 113:31:89: E501 Line too long (90 > 88)
+ doc/source/user_guide/style.ipynb:cell 113:33:89: E501 Line too long (96 > 88)
+ doc/source/user_guide/style.ipynb:cell 123:3:56: E741 Ambiguous variable name: `l`
+ doc/source/user_guide/style.ipynb:cell 125:2:13: C408 Unnecessary `dict` call (rewrite as a literal)
+ doc/source/user_guide/style.ipynb:cell 125:4:13: C408 Unnecessary `dict` call (rewrite as a literal)
+ doc/source/user_guide/style.ipynb:cell 125:6:13: C408 Unnecessary `dict` call (rewrite as a literal)
+ doc/source/user_guide/style.ipynb:cell 125:8:13: C408 Unnecessary `dict` call (rewrite as a literal)
+ doc/source/user_guide/style.ipynb:cell 126:1:1: NPY002 Replace legacy `np.random.seed` call with `np.random.Generator`
+ doc/source/user_guide/style.ipynb:cell 126:2:1: PLW0127 Self-assignment of variable `cmap`
+ doc/source/user_guide/style.ipynb:cell 126:2:8: PLW0128 Redeclared variable `cmap` in assignment
... 23 additional changes omitted for project

Changes by rule (70 rules affected)

code total + violation - violation + fix - fix
Q000 539 539 0 0 0
T201 151 151 0 0 0
E501 74 74 0 0 0
ANN001 62 62 0 0 0
NPY002 32 32 0 0 0
D103 25 25 0 0 0
W293 23 23 0 0 0
ANN201 22 22 0 0 0
FBT003 17 17 0 0 0
I001 12 12 0 0 0
F401 12 12 0 0 0
W291 11 11 0 0 0
COM812 11 11 0 0 0
E402 7 7 0 0 0
PTH123 6 6 0 0 0
C408 6 6 0 0 0
F821 5 5 0 0 0
ERA001 5 5 0 0 0
ANN202 5 5 0 0 0
E701 5 5 0 0 0
PD011 4 4 0 0 0
PD901 4 4 0 0 0
PLR2004 4 4 0 0 0
UP031 4 4 0 0 0
B007 3 3 0 0 0
T203 3 3 0 0 0
SLF001 3 3 0 0 0
RUF001 3 3 0 0 0
PTH110 3 3 0 0 0
E741 2 2 0 0 0
PLR1736 2 2 0 0 0
PLW2901 2 2 0 0 0
I002 2 2 0 0 0
PD002 2 2 0 0 0
PLW0602 2 2 0 0 0
ARG001 2 2 0 0 0
E721 1 1 0 0 0
B008 1 1 0 0 0
TID253 1 1 0 0 0
SIM115 1 1 0 0 0
S301 1 1 0 0 0
PLR0913 1 1 0 0 0
BLE001 1 1 0 0 0
RUF010 1 1 0 0 0
PD008 1 1 0 0 0
S113 1 1 0 0 0
PTH111 1 1 0 0 0
PTH202 1 1 0 0 0
PTH102 1 1 0 0 0
FBT001 1 1 0 0 0
D400 1 1 0 0 0
D415 1 1 0 0 0
PTH107 1 1 0 0 0
C405 1 1 0 0 0
B018 1 1 0 0 0
N803 1 1 0 0 0
C901 1 1 0 0 0
SIM910 1 1 0 0 0
E711 1 1 0 0 0
RUF005 1 1 0 0 0
D205 1 1 0 0 0
D212 1 1 0 0 0
F841 1 1 0 0 0
SIM108 1 1 0 0 0
UP030 1 1 0 0 0
UP032 1 1 0 0 0
S506 1 1 0 0 0
PLW0127 1 1 0 0 0
PLW0128 1 1 0 0 0
F811 1 1 0 0 0

Linter (preview)

✅ ecosystem check detected no linter changes.

@dhruvmanila dhruvmanila force-pushed the dhruv/include-notebooks-by-default branch 2 times, most recently from e1f238f to 2915388 Compare August 14, 2024 10:30
@dhruvmanila dhruvmanila added the configuration Related to settings and configuration label Aug 14, 2024
@dhruvmanila dhruvmanila force-pushed the dhruv/include-notebooks-by-default branch from 2915388 to 1d5be1c Compare August 14, 2024 10:41
@MichaReiser
Copy link
Member

Can you rebase/re-run the ecosystem checks. The augmented assignment adds a lot of noise.

Comment on lines 344 to 345
To lint or format files with additional file extensions, use the [`extend-include`](settings.md#extend-include) setting.

=== "pyproject.toml"

```toml
[tool.ruff]
extend-include = ["*.ipynb"]
```

=== "ruff.toml"

```toml
extend-include = ["*.ipynb"]
```

You can also change the default selection using the [`include`](settings.md#include) setting.
Copy link
Member

Choose a reason for hiding this comment

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

extend-include has an example with .pyw files. Although I think the example isn't needed because we already link to extend-include which has an example.

docs/configuration.md Outdated Show resolved Hide resolved
@MichaReiser
Copy link
Member

There are quiet a large number of violations and some violations seem less appropriate for notebooks (e.g. denying the use of print).

@dhruvmanila
Copy link
Member Author

Can you rebase/re-run the ecosystem checks. The augmented assignment adds a lot of noise.

It it up-to date with ruff-0.6 branch.

@MichaReiser
Copy link
Member

Let's see if re-triggering the ecosystem checks help

@MichaReiser MichaReiser reopened this Aug 14, 2024
@dhruvmanila
Copy link
Member Author

Let's see if re-triggering the ecosystem checks help

Oh, I see what the problem is. The ruff-0.6 branch needs to be rebased on main to remove the merge conflict. The CI is paused in that branch which means it never ran after the reverted commit.

@dhruvmanila
Copy link
Member Author

@MichaReiser I've made some edits as per your suggestions. Thanks.

I've also updated some rule documentation to reflect the different behavior for Jupyter Notebooks.

@dhruvmanila dhruvmanila force-pushed the dhruv/include-notebooks-by-default branch from 5421453 to 600dcc7 Compare August 14, 2024 12:22
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
docs/configuration.md Outdated Show resolved Hide resolved
@AlexWaygood AlexWaygood force-pushed the dhruv/include-notebooks-by-default branch from 0369ce8 to 051d8bd Compare August 14, 2024 14:58
@AlexWaygood
Copy link
Member

@MichaReiser, I pushed a few small updates. Would you mind having another quick look?

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.

Looks good except the options documentation is a bit of a stretch ;)

@AlexWaygood AlexWaygood force-pushed the dhruv/include-notebooks-by-default branch from 051d8bd to b53746c Compare August 14, 2024 15:04
@AlexWaygood AlexWaygood force-pushed the dhruv/include-notebooks-by-default branch from b53746c to 5a5a709 Compare August 14, 2024 15:15
@MichaReiser MichaReiser reopened this Aug 14, 2024
@AlexWaygood AlexWaygood merged commit 524a86a into ruff-0.6 Aug 14, 2024
39 checks passed
@AlexWaygood AlexWaygood deleted the dhruv/include-notebooks-by-default branch August 14, 2024 16:01
@AlexWaygood AlexWaygood mentioned this pull request Aug 14, 2024
MichaReiser pushed a commit that referenced this pull request Aug 14, 2024
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Closes: #12456
Closes: astral-sh/ruff-vscode#546
MichaReiser pushed a commit that referenced this pull request Aug 14, 2024
Co-authored-by: Alex Waygood <alex.waygood@gmail.com>
Closes: #12456
Closes: astral-sh/ruff-vscode#546
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking Breaking API change configuration Related to settings and configuration notebook Related to (Jupyter) notebooks
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants