Skip to content

Commit 0b6dab5

Browse files
authored
Add jupyter notebook cell ids in 4.5+ if missing (#6853)
**Summary** See #6834 (comment) **Test Plan** Added a new notebook
1 parent 1c66bb8 commit 0b6dab5

File tree

5 files changed

+92
-9
lines changed

5 files changed

+92
-9
lines changed

Cargo.lock

Lines changed: 22 additions & 2 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -50,6 +50,7 @@ tracing = "0.1.37"
5050
tracing-indicatif = "0.3.4"
5151
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
5252
unicode-width = "0.1.10"
53+
uuid = { version = "1.4.1", features = ["v4", "fast-rng", "macro-diagnostics", "js"] }
5354
wsl = { version = "0.1.0" }
5455

5556
# v1.0.1

crates/ruff/Cargo.toml

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -77,6 +77,7 @@ toml = { workspace = true }
7777
typed-arena = { version = "2.0.2" }
7878
unicode-width = { workspace = true }
7979
unicode_names2 = { version = "0.6.0", git = "https://github.com/youknowone/unicode_names2.git", rev = "4ce16aa85cbcdd9cc830410f1a72ef9a235f2fde" }
80+
uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics", "js"] }
8081
wsl = { version = "0.1.0" }
8182

8283
[dev-dependencies]
Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,37 @@
1+
{
2+
"cells": [
3+
{
4+
"cell_type": "code",
5+
"execution_count": null,
6+
"metadata": {},
7+
"outputs": [],
8+
"source": [
9+
"import math\n",
10+
"import os\n",
11+
"\n",
12+
"math.pi"
13+
]
14+
}
15+
],
16+
"metadata": {
17+
"kernelspec": {
18+
"display_name": "Python (ruff)",
19+
"language": "python",
20+
"name": "ruff"
21+
},
22+
"language_info": {
23+
"codemirror_mode": {
24+
"name": "ipython",
25+
"version": 3
26+
},
27+
"file_extension": ".py",
28+
"mimetype": "text/x-python",
29+
"name": "python",
30+
"nbconvert_exporter": "python",
31+
"pygments_lexer": "ipython3",
32+
"version": "3.11.3"
33+
}
34+
},
35+
"nbformat": 4,
36+
"nbformat_minor": 5
37+
}

crates/ruff/src/jupyter/notebook.rs

Lines changed: 31 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ use itertools::Itertools;
99
use once_cell::sync::OnceCell;
1010
use serde::Serialize;
1111
use serde_json::error::Category;
12+
use uuid::Uuid;
1213

1314
use ruff_diagnostics::Diagnostic;
1415
use ruff_python_parser::lexer::lex;
@@ -156,7 +157,7 @@ impl Notebook {
156157
TextRange::default(),
157158
)
158159
})?;
159-
let raw_notebook: RawNotebook = match serde_json::from_reader(reader.by_ref()) {
160+
let mut raw_notebook: RawNotebook = match serde_json::from_reader(reader.by_ref()) {
160161
Ok(notebook) => notebook,
161162
Err(err) => {
162163
// Translate the error into a diagnostic
@@ -262,6 +263,23 @@ impl Notebook {
262263
cell_offsets.push(current_offset);
263264
}
264265

266+
// Add cell ids to 4.5+ notebooks if they are missing
267+
// https://github.com/astral-sh/ruff/issues/6834
268+
// https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md#required-field
269+
if raw_notebook.nbformat == 4 && raw_notebook.nbformat_minor >= 5 {
270+
for cell in &mut raw_notebook.cells {
271+
let id = match cell {
272+
Cell::Code(cell) => &mut cell.id,
273+
Cell::Markdown(cell) => &mut cell.id,
274+
Cell::Raw(cell) => &mut cell.id,
275+
};
276+
if id.is_none() {
277+
// https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md#questions
278+
*id = Some(Uuid::new_v4().to_string());
279+
}
280+
}
281+
}
282+
265283
Ok(Self {
266284
raw: raw_notebook,
267285
index: OnceCell::new(),
@@ -662,21 +680,27 @@ print("after empty cells")
662680
Ok(())
663681
}
664682

665-
#[test]
666-
fn test_no_cell_id() -> Result<()> {
667-
let path = "no_cell_id.ipynb".to_string();
668-
let source_notebook = read_jupyter_notebook(path.as_ref())?;
683+
// Version <4.5, don't emit cell ids
684+
#[test_case(Path::new("no_cell_id.ipynb"), false; "no_cell_id")]
685+
// Version 4.5, cell ids are missing and need to be added
686+
#[test_case(Path::new("add_missing_cell_id.ipynb"), true; "add_missing_cell_id")]
687+
fn test_cell_id(path: &Path, has_id: bool) -> Result<()> {
688+
let source_notebook = read_jupyter_notebook(path)?;
669689
let source_kind = SourceKind::IpyNotebook(source_notebook);
670690
let (_, transformed) = test_contents(
671691
&source_kind,
672-
path.as_ref(),
692+
path,
673693
&settings::Settings::for_rule(Rule::UnusedImport),
674694
);
675695
let linted_notebook = transformed.into_owned().expect_ipy_notebook();
676696
let mut writer = Vec::new();
677697
linted_notebook.write_inner(&mut writer)?;
678698
let actual = String::from_utf8(writer)?;
679-
assert!(!actual.contains(r#""id":"#));
699+
if has_id {
700+
assert!(actual.contains(r#""id": ""#));
701+
} else {
702+
assert!(!actual.contains(r#""id":"#));
703+
}
680704
Ok(())
681705
}
682706
}

0 commit comments

Comments
 (0)