Skip to content

Commit

Permalink
Add jupyter notebook cell ids in 4.5+ if missing (#6853)
Browse files Browse the repository at this point in the history
**Summary** See
#6834 (comment)

**Test Plan** Added a new notebook
  • Loading branch information
konstin committed Aug 25, 2023
1 parent 1c66bb8 commit 0b6dab5
Show file tree
Hide file tree
Showing 5 changed files with 92 additions and 9 deletions.
24 changes: 22 additions & 2 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ tracing = "0.1.37"
tracing-indicatif = "0.3.4"
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
unicode-width = "0.1.10"
uuid = { version = "1.4.1", features = ["v4", "fast-rng", "macro-diagnostics", "js"] }
wsl = { version = "0.1.0" }

# v1.0.1
Expand Down
1 change: 1 addition & 0 deletions crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ toml = { workspace = true }
typed-arena = { version = "2.0.2" }
unicode-width = { workspace = true }
unicode_names2 = { version = "0.6.0", git = "https://github.com/youknowone/unicode_names2.git", rev = "4ce16aa85cbcdd9cc830410f1a72ef9a235f2fde" }
uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics", "js"] }
wsl = { version = "0.1.0" }

[dev-dependencies]
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,37 @@
{
"cells": [
{
"cell_type": "code",
"execution_count": null,
"metadata": {},
"outputs": [],
"source": [
"import math\n",
"import os\n",
"\n",
"math.pi"
]
}
],
"metadata": {
"kernelspec": {
"display_name": "Python (ruff)",
"language": "python",
"name": "ruff"
},
"language_info": {
"codemirror_mode": {
"name": "ipython",
"version": 3
},
"file_extension": ".py",
"mimetype": "text/x-python",
"name": "python",
"nbconvert_exporter": "python",
"pygments_lexer": "ipython3",
"version": "3.11.3"
}
},
"nbformat": 4,
"nbformat_minor": 5
}
38 changes: 31 additions & 7 deletions crates/ruff/src/jupyter/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ use itertools::Itertools;
use once_cell::sync::OnceCell;
use serde::Serialize;
use serde_json::error::Category;
use uuid::Uuid;

use ruff_diagnostics::Diagnostic;
use ruff_python_parser::lexer::lex;
Expand Down Expand Up @@ -156,7 +157,7 @@ impl Notebook {
TextRange::default(),
)
})?;
let raw_notebook: RawNotebook = match serde_json::from_reader(reader.by_ref()) {
let mut raw_notebook: RawNotebook = match serde_json::from_reader(reader.by_ref()) {
Ok(notebook) => notebook,
Err(err) => {
// Translate the error into a diagnostic
Expand Down Expand Up @@ -262,6 +263,23 @@ impl Notebook {
cell_offsets.push(current_offset);
}

// Add cell ids to 4.5+ notebooks if they are missing
// https://github.com/astral-sh/ruff/issues/6834
// https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md#required-field
if raw_notebook.nbformat == 4 && raw_notebook.nbformat_minor >= 5 {
for cell in &mut raw_notebook.cells {
let id = match cell {
Cell::Code(cell) => &mut cell.id,
Cell::Markdown(cell) => &mut cell.id,
Cell::Raw(cell) => &mut cell.id,
};
if id.is_none() {
// https://github.com/jupyter/enhancement-proposals/blob/master/62-cell-id/cell-id.md#questions
*id = Some(Uuid::new_v4().to_string());
}
}
}

Ok(Self {
raw: raw_notebook,
index: OnceCell::new(),
Expand Down Expand Up @@ -662,21 +680,27 @@ print("after empty cells")
Ok(())
}

#[test]
fn test_no_cell_id() -> Result<()> {
let path = "no_cell_id.ipynb".to_string();
let source_notebook = read_jupyter_notebook(path.as_ref())?;
// Version <4.5, don't emit cell ids
#[test_case(Path::new("no_cell_id.ipynb"), false; "no_cell_id")]
// Version 4.5, cell ids are missing and need to be added
#[test_case(Path::new("add_missing_cell_id.ipynb"), true; "add_missing_cell_id")]
fn test_cell_id(path: &Path, has_id: bool) -> Result<()> {
let source_notebook = read_jupyter_notebook(path)?;
let source_kind = SourceKind::IpyNotebook(source_notebook);
let (_, transformed) = test_contents(
&source_kind,
path.as_ref(),
path,
&settings::Settings::for_rule(Rule::UnusedImport),
);
let linted_notebook = transformed.into_owned().expect_ipy_notebook();
let mut writer = Vec::new();
linted_notebook.write_inner(&mut writer)?;
let actual = String::from_utf8(writer)?;
assert!(!actual.contains(r#""id":"#));
if has_id {
assert!(actual.contains(r#""id": ""#));
} else {
assert!(!actual.contains(r#""id":"#));
}
Ok(())
}
}

0 comments on commit 0b6dab5

Please sign in to comment.