From 9f71a12fc02070c855c7746b179869596b846f41 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 29 Nov 2023 15:22:11 -0600 Subject: [PATCH 1/2] Allow transparent cell magics --- .../fixtures/jupyter/cell/cell_magic.json | 7 ++- .../jupyter/cell/valid_cell_magic.json | 11 +++++ crates/ruff_notebook/src/cell.rs | 45 ++++++++++++++++++- crates/ruff_notebook/src/notebook.rs | 1 + 4 files changed, 62 insertions(+), 2 deletions(-) create mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/cell/valid_cell_magic.json diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/cell_magic.json b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/cell_magic.json index ef68b202e6811..e0de8c0241141 100644 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/cell_magic.json +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/cell_magic.json @@ -4,5 +4,10 @@ "id": "1", "metadata": {}, "outputs": [], - "source": ["%%timeit\n", "print('hello world')"] + "source": [ + "%%script bash\n", + "for i in 1 2 3; do\n", + " echo $i\n", + "done" + ] } diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/valid_cell_magic.json b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/valid_cell_magic.json new file mode 100644 index 0000000000000..2cb89fa63b9bd --- /dev/null +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/cell/valid_cell_magic.json @@ -0,0 +1,11 @@ +{ + "execution_count": null, + "cell_type": "code", + "id": "1", + "metadata": {}, + "outputs": [], + "source": [ + "%%timeit\n", + "print('hello world')" + ] +} diff --git a/crates/ruff_notebook/src/cell.rs b/crates/ruff_notebook/src/cell.rs index d80ef336de02e..caa4cb3204a03 100644 --- a/crates/ruff_notebook/src/cell.rs +++ b/crates/ruff_notebook/src/cell.rs @@ -170,7 +170,50 @@ impl Cell { } // Detect cell magics (which operate on multiple lines). - lines.any(|line| line.trim_start().starts_with("%%")) + lines.any(|line| { + line.split_whitespace().next().is_some_and(|first| { + if first.len() < 2 { + return false; + } + let (token, command) = first.split_at(2); + // These cell magics are special in that the lines following them are valid + // Python code and the variables defined in that scope are available to the + // rest of the notebook. + // + // For example: + // + // Cell 1: + // ```python + // x = 1 + // ``` + // + // Cell 2: + // ```python + // %%time + // y = x + // ``` + // + // Cell 3: + // ```python + // print(y) # Here, `y` is available. + // ``` + // + // This is to avoid false positives when these variables are referenced + // elsewhere in the notebook. + token == "%%" + && !matches!( + command, + "capture" + | "debug" + | "prun" + | "pypy" + | "python" + | "python3" + | "time" + | "timeit" + ) + }) + }) } } diff --git a/crates/ruff_notebook/src/notebook.rs b/crates/ruff_notebook/src/notebook.rs index a6714fa12b496..7c9e8356b79d3 100644 --- a/crates/ruff_notebook/src/notebook.rs +++ b/crates/ruff_notebook/src/notebook.rs @@ -426,6 +426,7 @@ mod tests { #[test_case(Path::new("code_and_magic.json"), true; "code_and_magic")] #[test_case(Path::new("only_code.json"), true; "only_code")] #[test_case(Path::new("cell_magic.json"), false; "cell_magic")] + #[test_case(Path::new("valid_cell_magic.json"), true; "valid_cell_magic")] #[test_case(Path::new("automagic.json"), false; "automagic")] #[test_case(Path::new("automagics.json"), false; "automagics")] #[test_case(Path::new("automagic_before_code.json"), false; "automagic_before_code")] From 11b767387333cafb2e1e42b754e611c2c294391c Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 29 Nov 2023 15:53:48 -0600 Subject: [PATCH 2/2] Update existing test case --- ...linter__linter__tests__ipy_escape_command.snap | 15 +++++++++++++++ .../fixtures/jupyter/ipy_escape_command.ipynb | 12 ++++++++++++ .../jupyter/ipy_escape_command_expected.ipynb | 15 +++++++++++++-- 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__ipy_escape_command.snap b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__ipy_escape_command.snap index 58aa72d7da26b..8c1a547e85966 100644 --- a/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__ipy_escape_command.snap +++ b/crates/ruff_linter/src/snapshots/ruff_linter__linter__tests__ipy_escape_command.snap @@ -19,5 +19,20 @@ ipy_escape_command.ipynb:cell 1:5:8: F401 [*] `os` imported but unused 5 |-import os 6 5 | 7 6 | _ = math.pi +8 7 | %%timeit + +ipy_escape_command.ipynb:cell 2:2:8: F401 [*] `sys` imported but unused + | +1 | %%timeit +2 | import sys + | ^^^ F401 + | + = help: Remove unused import: `sys` + +ℹ Safe fix +6 6 | +7 7 | _ = math.pi +8 8 | %%timeit +9 |-import sys diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command.ipynb index 5e9b10bb7b0e1..6937096cc0a51 100644 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command.ipynb +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command.ipynb @@ -26,6 +26,18 @@ "%%timeit\n", "import sys" ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "36dedfd1-6c03-4894-bea6-6c1687b82b3c", + "metadata": {}, + "outputs": [], + "source": [ + "%%random\n", + "# This cell is ignored\n", + "import pathlib" + ] } ], "metadata": { diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb index 8419f031e78f8..6a5eebc05fa80 100644 --- a/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/ipy_escape_command_expected.ipynb @@ -22,8 +22,19 @@ "metadata": {}, "outputs": [], "source": [ - "%%timeit\n", - "import sys" + "%%timeit" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "id": "4b6d7faa-72b3-4087-8670-fe6d35e41fb6", + "metadata": {}, + "outputs": [], + "source": [ + "%%random\n", + "# This cell is ignored\n", + "import pathlib" ] } ],