From 28db6da0fdfac3502201a2a6e18c70d257b2ed86 Mon Sep 17 00:00:00 2001 From: Dhruv Manilawala Date: Wed, 14 Aug 2024 08:49:56 +0530 Subject: [PATCH] Fallback to kernelspec to check if it's a Python notebook --- .../jupyter/kernelspec_language.ipynb | 48 +++++++++++++++++++ crates/ruff_notebook/src/notebook.rs | 37 +++++++------- crates/ruff_notebook/src/schema.rs | 23 +++++++-- 3 files changed, 85 insertions(+), 23 deletions(-) create mode 100644 crates/ruff_notebook/resources/test/fixtures/jupyter/kernelspec_language.ipynb diff --git a/crates/ruff_notebook/resources/test/fixtures/jupyter/kernelspec_language.ipynb b/crates/ruff_notebook/resources/test/fixtures/jupyter/kernelspec_language.ipynb new file mode 100644 index 00000000000000..92abff0e427bb8 --- /dev/null +++ b/crates/ruff_notebook/resources/test/fixtures/jupyter/kernelspec_language.ipynb @@ -0,0 +1,48 @@ +{ + "cells": [ + { + "cell_type": "markdown", + "metadata": {}, + "source": [ + "# VS Code `languageId`\n", + "\n", + "This is a test notebook for VS Code specific cell metadata.\n" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": { + "vscode": { + "languageId": "javascript" + } + }, + "outputs": [], + "source": [ + "function add(x, y) {\n", + " return x + y;\n", + "}" + ] + }, + { + "cell_type": "code", + "execution_count": null, + "metadata": {}, + "outputs": [], + "source": [ + "import os\n", + "\n", + "print(\"hello world\")" + ] + } + ], + "metadata": { + "kernelspec": { + "display_name": "Python 3", + "language": "python", + "name": "python" + } + }, + "nbformat": 4, + "nbformat_minor": 2 +} diff --git a/crates/ruff_notebook/src/notebook.rs b/crates/ruff_notebook/src/notebook.rs index b2be9ebe6ae507..c04b3557e033b7 100644 --- a/crates/ruff_notebook/src/notebook.rs +++ b/crates/ruff_notebook/src/notebook.rs @@ -410,11 +410,13 @@ impl Notebook { /// Return `true` if the notebook is a Python notebook, `false` otherwise. pub fn is_python_notebook(&self) -> bool { - self.raw - .metadata - .language_info - .as_ref() - .map_or(true, |language| language.name == "python") + if let Some(language_info) = self.raw.metadata.language_info.as_ref() { + return language_info.name == "python"; + } + if let Some(kernel_spec) = self.raw.metadata.kernelspec.as_ref() { + return kernel_spec.language.as_deref() == Some("python"); + } + false } /// Write the notebook back to the given [`Write`] implementer. @@ -456,18 +458,12 @@ mod tests { Path::new("./resources/test/fixtures/jupyter").join(path) } - #[test] - fn test_python() -> Result<(), NotebookError> { - let notebook = Notebook::from_path(¬ebook_path("valid.ipynb"))?; - assert!(notebook.is_python_notebook()); - Ok(()) - } - - #[test] - fn test_r() -> Result<(), NotebookError> { - let notebook = Notebook::from_path(¬ebook_path("R.ipynb"))?; - assert!(!notebook.is_python_notebook()); - Ok(()) + #[test_case("valid.ipynb", true)] + #[test_case("R.ipynb", false)] + #[test_case("kernelspec_language.ipynb", true)] + fn is_python_notebook(filename: &str, expected: bool) { + let notebook = Notebook::from_path(¬ebook_path(filename)).unwrap(); + assert_eq!(notebook.is_python_notebook(), expected); } #[test] @@ -597,9 +593,10 @@ print("after empty cells") Ok(()) } - #[test] - fn round_trip() { - let path = notebook_path("vscode_language_id.ipynb"); + #[test_case("vscode_language_id.ipynb")] + #[test_case("kernelspec_language.ipynb")] + fn round_trip(filename: &str) { + let path = notebook_path(filename); let expected = std::fs::read_to_string(&path).unwrap(); let actual = super::round_trip(&path).unwrap(); assert_eq!(actual, expected); diff --git a/crates/ruff_notebook/src/schema.rs b/crates/ruff_notebook/src/schema.rs index a33d041055dfc8..d48b7483fedf34 100644 --- a/crates/ruff_notebook/src/schema.rs +++ b/crates/ruff_notebook/src/schema.rs @@ -169,7 +169,7 @@ pub struct CellMetadata { /// preferred language. /// pub vscode: Option, - /// Catch-all for metadata that isn't required by Ruff. + /// For additional properties that isn't required by Ruff. #[serde(flatten)] pub extra: HashMap, } @@ -190,8 +190,8 @@ pub struct RawNotebookMetadata { /// The author(s) of the notebook document pub authors: Option, /// Kernel information. - pub kernelspec: Option, - /// Kernel information. + pub kernelspec: Option, + /// Language information. pub language_info: Option, /// Original notebook format (major number) before converting the notebook between versions. /// This should never be written to a file. @@ -206,6 +206,23 @@ pub struct RawNotebookMetadata { /// Kernel information. #[skip_serializing_none] #[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] +pub struct Kernelspec { + /// The language name. This isn't mentioned in the spec but is populated by various tools and + /// can be used as a fallback if [`language_info`] is missing. + /// + /// This is also used by VS Code to determine the preferred language of the notebook: + /// . + /// + /// [`language_info`]: RawNotebookMetadata::language_info + pub language: Option, + /// For additional properties that isn't required by Ruff. + #[serde(flatten)] + pub extra: HashMap, +} + +/// Language information. +#[skip_serializing_none] +#[derive(Clone, Debug, Serialize, Deserialize, PartialEq)] pub struct LanguageInfo { /// The codemirror mode to use for code in this language. pub codemirror_mode: Option,