Skip to content

Commit

Permalink
Add jupyter notebook cell ids in 4.5+ if missing
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 24, 2023
1 parent 8a031c4 commit bae42c4
Show file tree
Hide file tree
Showing 5 changed files with 93 additions and 13 deletions.
21 changes: 19 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 @@ -49,6 +49,7 @@ toml = { version = "0.7.2" }
tracing = "0.1.37"
tracing-indicatif = "0.3.4"
tracing-subscriber = { version = "0.3.17", features = ["env-filter"] }
uuid = { version = "1.4.1", features = ["v4", "fast-rng", "macro-diagnostics"] }
unicode-width = "0.1.10"
wsl = { version = "0.1.0" }

Expand Down
1 change: 1 addition & 0 deletions crates/ruff/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ strum_macros = { workspace = true }
thiserror = { version = "1.0.43" }
toml = { workspace = true }
typed-arena = { version = "2.0.2" }
uuid = { workspace = true, features = ["v4", "fast-rng", "macro-diagnostics"] }
unicode-width = { workspace = true }
unicode_names2 = { version = "0.6.0", git = "https://github.com/youknowone/unicode_names2.git", rev = "4ce16aa85cbcdd9cc830410f1a72ef9a235f2fde" }
wsl = { version = "0.1.0" }
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
}
46 changes: 35 additions & 11 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 @@ -293,11 +311,11 @@ impl Notebook {
.iter()
.rev()
.find(|m| m.source <= *offset)
else {
// There are no markers above the current offset, so we can
// stop here.
break;
};
else {
// There are no markers above the current offset, so we can
// stop here.
break;
};
last_marker = Some(marker);
marker
}
Expand All @@ -313,6 +331,8 @@ impl Notebook {

/// Update the cell contents with the transformed content.
///
/// Also inserts cell ids for jupyter notebook version 4.5+ where they are missing
///
/// ## Panics
///
/// Panics if the transformed content is out of bounds for any cell. This
Expand Down Expand Up @@ -662,10 +682,10 @@ 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())?;
#[test_case(Path::new("no_cell_id.ipynb"), false; "no_cell_id")]
#[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::Jupyter(source_notebook);
let (_, transformed) = test_contents(
&source_kind,
Expand All @@ -676,7 +696,11 @@ print("after empty cells")
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 bae42c4

Please sign in to comment.