-
Notifications
You must be signed in to change notification settings - Fork 1.1k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Consider VS Code cell metadata to determine valid code cells #12864
Conversation
8e977dd
to
11c222c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Awesome, thank you.
I think it would be good to add a round-trip test case with additional CellMetadata
.
CodSpeed Performance ReportMerging #12864 will degrade performances by 4.09%Comparing Summary
Benchmarks breakdown
|
|
Added a simple roundtrip test case with additional cell metadata. I also tested it using |
## Summary Follow-up to #12864, we don't need to exclude these notebooks anymore. ## Test plan - [x] Make sure that ecosystem checks are green.
## Summary Related to astral-sh/ruff#12864, the cell metadata is actually a required field. We got away with it previously because Ruff used `Value`.
Summary
This PR adds support for VS Code specific cell metadata to consider when collecting valid code cells.
For context, Ruff only runs on valid code cells. These are the code cells that doesn't contain cell magics. Previously, Ruff only used the notebook's metadata to determine whether it's a Python notebook. But, in VS Code, a notebook's preferred language might be Python but it could still contain code cells for other languages. This can be determined with the
metadata.vscode.languageId
field.References:
This brings us one step closer to fixing #12281.
Test Plan
Add test cases for
is_valid_python_code_cell
and an integration test case which showcase running it end to end. The test notebook contains a JavaScript code cell and a Python code cell.