Conversation
|
Current dependencies on/for this PR:
This stack of pull requests is managed by Graphite. |
cc82cff to
903e860
Compare
| // Cell 2: | ||
| // ```python | ||
| // %%time | ||
| // y = x |
There was a problem hiding this comment.
So how does the parser handle this case?
There was a problem hiding this comment.
The parser would parse the first line %%time as a cell magic statement and the second line as an assignment statement.
This is technically a bug in the parser as both lines should be a single node instead but as we always ignored cell magic, it never came up.
There was a problem hiding this comment.
Where does the cell magic end? Does it just include the %%time regardless of what follows? Like how would the parser deal with:
%%time x = 1
y = xThere was a problem hiding this comment.
Each statement is a single node, so %%time x = 1 would a single node and y = x would be an assignment statement. This code would raise undefined name for x. This is intentional as that requires support for inline parsing which might require more thinking.
There was a problem hiding this comment.
I'm mostly wondering how the parser knows where the magic statement ends.
There was a problem hiding this comment.
Oh, it's the newline. The lexer consumes everything till the newline and considers this as part of the command.
There was a problem hiding this comment.
Ah okay, makes sense. Could there be continuations though?
There was a problem hiding this comment.
Yeah, but that is ignored. So,
%%time \
x = 1
y = xwill parse in same way as your example.
|
|
@dhruvmanila - What are your thoughts on moving this one forward? Are there concerns? |
d8520c4 to
11b7673
Compare

Summary
This PR updates the logic for
is_magic_cellto include certain cell magics. These cell magics would contain Python code following the line defining the command. The code could define a variable which can then be referenced in other cells. Currently, we would ignore the cell completely leading to undefined-name violation.As discussed in #8354 (comment)
Test Plan
Add new test case to validate this scenario.