Skip to content

Commit

Permalink
Rename Magic* to IpyEscape* (#6395)
Browse files Browse the repository at this point in the history
## Summary

This PR renames the `MagicCommand` token to `IpyEscapeCommand` token and
`MagicKind` to `IpyEscapeKind` type to better reflect the purpose of the
token and type. Similarly, it renames the AST nodes from `LineMagic` to
`IpyEscapeCommand` prefixed with `Stmt`/`Expr` wherever necessary.

It also makes renames from using `jupyter_magic` to
`ipython_escape_commands` in various function names.

The mode value is still `Mode::Jupyter` because the escape commands are
part of the IPython syntax but the lexing/parsing is done for a Jupyter
notebook.

### Motivation behind the rename:
* IPython codebase defines it as "EscapeCommand" / "Escape Sequences":
* Escape Sequences:
https://github.com/ipython/ipython/blob/292e3a23459ca965b8c1bfe2c3707044c510209a/IPython/core/inputtransformer2.py#L329-L333
* Escape command:
https://github.com/ipython/ipython/blob/292e3a23459ca965b8c1bfe2c3707044c510209a/IPython/core/inputtransformer2.py#L410-L411
* The word "magic" is used mainly for the actual magic commands i.e.,
the ones starting with `%`/`%%`
(https://ipython.readthedocs.io/en/stable/interactive/reference.html#magic-command-system).
So, this avoids any confusion between the Magic token (`%`, `%%`) and
the escape command itself.
## Test Plan

* `cargo test` to make sure all renames are done correctly.
* `grep` for `jupyter_escape`/`magic` to make sure all renames are done
correctly.
  • Loading branch information
dhruvmanila authored Aug 9, 2023
1 parent 3bf1c66 commit 6a64f22
Show file tree
Hide file tree
Showing 26 changed files with 867 additions and 864 deletions.
6 changes: 3 additions & 3 deletions crates/ruff/src/jupyter/notebook.rs
Original file line number Diff line number Diff line change
Expand Up @@ -571,11 +571,11 @@ print("after empty cells")
}

#[test]
fn test_line_magics() -> Result<()> {
let path = "line_magics.ipynb".to_string();
fn test_ipy_escape_command() -> Result<()> {
let path = "ipy_escape_command.ipynb".to_string();
let (diagnostics, source_kind, _) = test_notebook_path(
&path,
Path::new("line_magics_expected.ipynb"),
Path::new("ipy_escape_command_expected.ipynb"),
&settings::Settings::for_rule(Rule::UnusedImport),
)?;
assert_messages!(diagnostics, path, source_kind);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
---
source: crates/ruff/src/jupyter/notebook.rs
---
line_magics.ipynb:cell 1:5:8: F401 [*] `os` imported but unused
ipy_escape_command.ipynb:cell 1:5:8: F401 [*] `os` imported but unused
|
3 | %matplotlib inline
4 |
Expand Down
8 changes: 4 additions & 4 deletions crates/ruff/src/rules/ruff/rules/unreachable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -653,13 +653,13 @@ impl<'stmt> BasicBlocksBuilder<'stmt> {
| Expr::Await(_)
| Expr::Yield(_)
| Expr::YieldFrom(_) => self.unconditional_next_block(after),
Expr::LineMagic(_) => todo!(),
Expr::IpyEscapeCommand(_) => todo!(),
}
}
// The tough branches are done, here is an easy one.
Stmt::Return(_) => NextBlock::Terminate,
Stmt::TypeAlias(_) => todo!(),
Stmt::LineMagic(_) => todo!(),
Stmt::IpyEscapeCommand(_) => todo!(),
};

// Include any statements in the block that don't divert the control flow.
Expand Down Expand Up @@ -903,7 +903,7 @@ fn needs_next_block(stmts: &[Stmt]) -> bool {
| Stmt::TryStar(_)
| Stmt::Assert(_) => true,
Stmt::TypeAlias(_) => todo!(),
Stmt::LineMagic(_) => todo!(),
Stmt::IpyEscapeCommand(_) => todo!(),
}
}

Expand Down Expand Up @@ -936,7 +936,7 @@ fn is_control_flow_stmt(stmt: &Stmt) -> bool {
| Stmt::Break(_)
| Stmt::Continue(_) => true,
Stmt::TypeAlias(_) => todo!(),
Stmt::LineMagic(_) => todo!(),
Stmt::IpyEscapeCommand(_) => todo!(),
}
}

Expand Down
20 changes: 10 additions & 10 deletions crates/ruff_python_ast/src/comparable.rs
Original file line number Diff line number Diff line change
Expand Up @@ -672,8 +672,8 @@ pub struct ExprSlice<'a> {
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct ExprLineMagic<'a> {
kind: ast::MagicKind,
pub struct ExprIpyEscapeCommand<'a> {
kind: ast::IpyEscapeKind,
value: &'a str,
}

Expand Down Expand Up @@ -706,7 +706,7 @@ pub enum ComparableExpr<'a> {
List(ExprList<'a>),
Tuple(ExprTuple<'a>),
Slice(ExprSlice<'a>),
LineMagic(ExprLineMagic<'a>),
IpyEscapeCommand(ExprIpyEscapeCommand<'a>),
}

impl<'a> From<&'a Box<ast::Expr>> for Box<ComparableExpr<'a>> {
Expand Down Expand Up @@ -936,11 +936,11 @@ impl<'a> From<&'a ast::Expr> for ComparableExpr<'a> {
upper: upper.as_ref().map(Into::into),
step: step.as_ref().map(Into::into),
}),
ast::Expr::LineMagic(ast::ExprLineMagic {
ast::Expr::IpyEscapeCommand(ast::ExprIpyEscapeCommand {
kind,
value,
range: _,
}) => Self::LineMagic(ExprLineMagic {
}) => Self::IpyEscapeCommand(ExprIpyEscapeCommand {
kind: *kind,
value: value.as_str(),
}),
Expand Down Expand Up @@ -1165,8 +1165,8 @@ pub struct StmtExpr<'a> {
}

#[derive(Debug, PartialEq, Eq, Hash)]
pub struct StmtLineMagic<'a> {
kind: ast::MagicKind,
pub struct StmtIpyEscapeCommand<'a> {
kind: ast::IpyEscapeKind,
value: &'a str,
}

Expand All @@ -1193,7 +1193,7 @@ pub enum ComparableStmt<'a> {
ImportFrom(StmtImportFrom<'a>),
Global(StmtGlobal<'a>),
Nonlocal(StmtNonlocal<'a>),
LineMagic(StmtLineMagic<'a>),
IpyEscapeCommand(StmtIpyEscapeCommand<'a>),
Expr(StmtExpr<'a>),
Pass,
Break,
Expand Down Expand Up @@ -1394,11 +1394,11 @@ impl<'a> From<&'a ast::Stmt> for ComparableStmt<'a> {
names: names.iter().map(ast::Identifier::as_str).collect(),
})
}
ast::Stmt::LineMagic(ast::StmtLineMagic {
ast::Stmt::IpyEscapeCommand(ast::StmtIpyEscapeCommand {
kind,
value,
range: _,
}) => Self::LineMagic(StmtLineMagic {
}) => Self::IpyEscapeCommand(StmtIpyEscapeCommand {
kind: *kind,
value: value.as_str(),
}),
Expand Down
6 changes: 3 additions & 3 deletions crates/ruff_python_ast/src/helpers.rs
Original file line number Diff line number Diff line change
Expand Up @@ -108,7 +108,7 @@ where
| Expr::Subscript(_)
| Expr::Yield(_)
| Expr::YieldFrom(_)
| Expr::LineMagic(_)
| Expr::IpyEscapeCommand(_)
)
})
}
Expand Down Expand Up @@ -247,7 +247,7 @@ where
.is_some_and(|value| any_over_expr(value, func))
}
Expr::Name(_) | Expr::Constant(_) => false,
Expr::LineMagic(_) => false,
Expr::IpyEscapeCommand(_) => false,
}
}

Expand Down Expand Up @@ -534,7 +534,7 @@ where
Stmt::Nonlocal(_) => false,
Stmt::Expr(ast::StmtExpr { value, range: _ }) => any_over_expr(value, func),
Stmt::Pass(_) | Stmt::Break(_) | Stmt::Continue(_) => false,
Stmt::LineMagic(_) => false,
Stmt::IpyEscapeCommand(_) => false,
}
}

Expand Down
Loading

0 comments on commit 6a64f22

Please sign in to comment.