From e1936858bfb15b1cbc662c1e288b4e3997b2cac6 Mon Sep 17 00:00:00 2001 From: harupy Date: Thu, 24 Aug 2023 21:11:32 +0900 Subject: [PATCH 1/4] Skip serializing cell ID if it's None --- crates/ruff/src/jupyter/schema.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/crates/ruff/src/jupyter/schema.rs b/crates/ruff/src/jupyter/schema.rs index b6f9ed3c471348..17b03e474a3128 100644 --- a/crates/ruff/src/jupyter/schema.rs +++ b/crates/ruff/src/jupyter/schema.rs @@ -120,6 +120,7 @@ pub struct RawCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// + #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value, @@ -135,6 +136,7 @@ pub struct MarkdownCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// + #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value, @@ -150,6 +152,7 @@ pub struct CodeCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// + #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value, From 3a3de94aadf1dcd66ebdfee31186dd66c70049be Mon Sep 17 00:00:00 2001 From: harupy Date: Thu, 24 Aug 2023 21:46:28 +0900 Subject: [PATCH 2/4] test --- .../test/fixtures/jupyter/no_cell_id.ipynb | 37 +++++++++++++++++++ crates/ruff/src/jupyter/notebook.rs | 22 ++++++++++- crates/ruff/src/test.rs | 2 +- 3 files changed, 59 insertions(+), 2 deletions(-) create mode 100644 crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb diff --git a/crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb b/crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb new file mode 100644 index 00000000000000..ea9e9e654af2b3 --- /dev/null +++ b/crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb @@ -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": 2 +} diff --git a/crates/ruff/src/jupyter/notebook.rs b/crates/ruff/src/jupyter/notebook.rs index 018dc2d864307b..144fa948c41498 100644 --- a/crates/ruff/src/jupyter/notebook.rs +++ b/crates/ruff/src/jupyter/notebook.rs @@ -477,8 +477,10 @@ mod tests { use crate::jupyter::schema::Cell; use crate::jupyter::Notebook; use crate::registry::Rule; + use crate::source_kind::SourceKind; use crate::test::{ - read_jupyter_notebook, test_notebook_path, test_resource_path, TestedNotebook, + read_jupyter_notebook, test_contents, test_notebook_path, test_resource_path, + TestedNotebook, }; use crate::{assert_messages, settings}; @@ -659,4 +661,22 @@ 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())?; + let source_kind = SourceKind::Jupyter(source_notebook); + let (_, transformed) = test_contents( + &source_kind, + path.as_ref(), + &settings::Settings::for_rule(Rule::UnusedImport), + ); + let linted_notebook = transformed.into_owned().expect_jupyter(); + let mut writer = Vec::new(); + linted_notebook.write_inner(&mut writer)?; + let actual = String::from_utf8(writer)?; + assert!(!actual.contains(r#""id":"#)); + Ok(()) + } } diff --git a/crates/ruff/src/test.rs b/crates/ruff/src/test.rs index 7434a4ef45a82d..5fa67c4eb74da3 100644 --- a/crates/ruff/src/test.rs +++ b/crates/ruff/src/test.rs @@ -112,7 +112,7 @@ pub(crate) fn max_iterations() -> usize { /// A convenient wrapper around [`check_path`], that additionally /// asserts that autofixes converge after a fixed number of iterations. -fn test_contents<'a>( +pub(crate) fn test_contents<'a>( source_kind: &'a SourceKind, path: &Path, settings: &Settings, From 24002880c959c3eefde96bd6915dc36d5f502c0f Mon Sep 17 00:00:00 2001 From: Harutaka Kawamura Date: Thu, 24 Aug 2023 21:54:07 +0900 Subject: [PATCH 3/4] Apply suggestions from code review Co-authored-by: Dhruv Manilawala --- crates/ruff/src/jupyter/schema.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/crates/ruff/src/jupyter/schema.rs b/crates/ruff/src/jupyter/schema.rs index 17b03e474a3128..18f4d3428f7116 100644 --- a/crates/ruff/src/jupyter/schema.rs +++ b/crates/ruff/src/jupyter/schema.rs @@ -120,7 +120,6 @@ pub struct RawCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// - #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value, @@ -136,7 +135,6 @@ pub struct MarkdownCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// - #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value, @@ -152,7 +150,7 @@ pub struct CodeCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// - #[serde(skip_serializing_if = "Option::is_none")] + #[serialize_always] pub id: Option, /// Cell-level metadata. pub metadata: Value, From 8a031c40aec4a9987ae68897731c1d199f9fa8e5 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Thu, 24 Aug 2023 18:39:10 +0530 Subject: [PATCH 4/4] Revert to using `skip_serializing_if` --- crates/ruff/src/jupyter/schema.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/crates/ruff/src/jupyter/schema.rs b/crates/ruff/src/jupyter/schema.rs index 18f4d3428f7116..e466615feca520 100644 --- a/crates/ruff/src/jupyter/schema.rs +++ b/crates/ruff/src/jupyter/schema.rs @@ -150,7 +150,7 @@ pub struct CodeCell { /// Technically, id isn't required (it's not even present) in schema v4.0 through v4.4, but /// it's required in v4.5. Main issue is that pycharm creates notebooks without an id /// - #[serialize_always] + #[serde(skip_serializing_if = "Option::is_none")] pub id: Option, /// Cell-level metadata. pub metadata: Value,