Fallback to kernelspec to check if it's a Python notebook #12875
Conversation
dhruvmanila
force-pushed
the
dhruv/kernel-spec
branch
from
28db6da to
5bed32f
Compare
crates/ruff_notebook/src/notebook.rs
Outdated
| 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 |
There was a problem hiding this comment.
I'm not exactly sure why was the default value true in case language_info is absent but that doesn't seem correct to me. If we can't infer the language from either language_info or kernelspec, we'll now return false instead.
There was a problem hiding this comment.
Ok, so the notebooks in the ecosystem diff doesn't have both language_info and kernelspec in which case we're not marking it as Python notebook. I think I'd need to revert this change to return true in this case.
|
|
Need to look at what's going on with the notebooks in ecosystem output. |
dhruvmanila
force-pushed
the
dhruv/kernel-spec
branch
from
5bed32f to
03d2c0c
Compare
There was a problem hiding this comment.
Awesome. Thanks @dhruvmanila for pushing this changes before the release!
Summary
This PR adds a fallback logic for
is_python_notebookto check thekernelspec.languagefield.Reference implementation in VS Code: https://github.com/microsoft/vscode/blob/1c31e758985efe11bc0453a45ea0bb6887e670a4/extensions/ipynb/src/deserializers.ts#L20-L22
It's also required for the kernel to provide the
languagethey're implementing based on https://jupyter-client.readthedocs.io/en/stable/kernels.html#kernel-specs reference although that's for thekernel.jsonfile but is also included in the notebook metadata.Closes: #12281
Test Plan
Add a test case for
is_python_notebookand include the test notebook for round trip validation.The test notebook contains two cells, one is JavaScript (denoted via the
vscode.languageIdmetadata) and the other is Python (no metadata). The notebook metadata only containskernelspecand thelanguage_infois absent.I also verified that this is a valid notebook by opening it in Jupyter Lab, VS Code and using
nbformatvalidator.