Skip to content

Commit

Permalink
Skip serializing cell ID if it's None (#6851)
Browse files Browse the repository at this point in the history
<!--
Thank you for contributing to Ruff! To help us out with reviewing,
please consider the following:

- Does this pull request include a summary of the change? (See below.)
- Does this pull request include a descriptive title?
- Does this pull request include references to any relevant issues?
-->

## Summary

<!-- What's the purpose of the change? What does it do, and why? -->

Fix #6834 

## Test Plan

<!-- How was it tested? -->

Need tests?

---------

Co-authored-by: Dhruv Manilawala <[email protected]>
  • Loading branch information
harupy and dhruvmanila authored Aug 24, 2023
1 parent 1e7d196 commit 948cd29
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 2 deletions.
37 changes: 37 additions & 0 deletions crates/ruff/resources/test/fixtures/jupyter/no_cell_id.ipynb
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": 2
}
22 changes: 21 additions & 1 deletion crates/ruff/src/jupyter/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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(())
}
}
1 change: 1 addition & 0 deletions crates/ruff/src/jupyter/schema.rs
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +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
/// <https://youtrack.jetbrains.com/issue/PY-59438/Jupyter-notebooks-created-with-PyCharm-are-missing-the-id-field-in-cells-in-the-.ipynb-json>
#[serde(skip_serializing_if = "Option::is_none")]
pub id: Option<String>,
/// Cell-level metadata.
pub metadata: Value,
Expand Down
2 changes: 1 addition & 1 deletion crates/ruff/src/test.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down

0 comments on commit 948cd29

Please sign in to comment.