Skip to content
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

Enable formatting for Jupyter notebooks #7749

Merged
merged 5 commits into from
Oct 2, 2023
Merged

Conversation

charliermarsh
Copy link
Member

Summary

This PR enables ruff format to format Jupyter notebooks.

Most of the work is contained in a new format_source method that formats a generic SourceKind, then returns Some(transformed) if the source required formatting, or None otherwise.

Closes #7598.

Test Plan

Ran cat foo.py | cargo run -p ruff_cli -- format --stdin-filename Untitled.ipynb; verified that the console showed a reasonable error:

warning: Failed to read notebook Untitled.ipynb: Expected a Jupyter Notebook, which must be internally stored as JSON, but this file isn't valid JSON: EOF while parsing a value at line 1 column 0

Ran cat Untitled.ipynb | cargo run -p ruff_cli -- format --stdin-filename Untitled.ipynb; verified that the JSON output contained formatted source code.

@charliermarsh charliermarsh added the formatter Related to the formatter label Oct 1, 2023
@charliermarsh charliermarsh force-pushed the charlie/format-jupyter branch 6 times, most recently from 2068ea6 to 21f58ab Compare October 2, 2023 00:03
@@ -487,80 +508,3 @@ pub(crate) fn lint_stdin(
notebook_indexes,
})
}

#[derive(Debug)]
pub(crate) struct LintSource(pub(crate) SourceKind);
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I removed this, and moved the logic into SourceKind, so that it can be used for formatting too.

@charliermarsh charliermarsh force-pushed the charlie/format-jupyter branch 2 times, most recently from c59207d to 8bcc8db Compare October 2, 2023 00:19
@github-actions
Copy link
Contributor

github-actions bot commented Oct 2, 2023

PR Check Results

Ecosystem

✅ ecosystem check detected no changes.

crates/ruff_cli/src/commands/format.rs Show resolved Hide resolved
crates/ruff_cli/src/commands/format.rs Outdated Show resolved Hide resolved
return Ok(None);
}

let mut output = String::with_capacity(notebook.source_code().len());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can avoid the allocation here by only reserving once we see the first formatted cell

crates/ruff_cli/src/commands/format.rs Show resolved Hide resolved
crates/ruff_cli/src/commands/format_stdin.rs Show resolved Hide resolved
Base automatically changed from charlie/write to main October 2, 2023 14:20
@charliermarsh charliermarsh force-pushed the charlie/format-jupyter branch from 8bcc8db to d97d454 Compare October 2, 2023 14:34
@charliermarsh charliermarsh enabled auto-merge (squash) October 2, 2023 14:39
@charliermarsh charliermarsh merged commit bdf2852 into main Oct 2, 2023
15 checks passed
@charliermarsh charliermarsh deleted the charlie/format-jupyter branch October 2, 2023 14:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
formatter Related to the formatter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Format jupyter notebooks
2 participants