-
Notifications
You must be signed in to change notification settings - Fork 65
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
🪓 Split Output
into Outputs -> Output[]
#1671
base: main
Are you sure you want to change the base?
Conversation
|
dba90de
to
ae6a292
Compare
// Embed outputs in an output block | ||
const result: { type: 'output'; id: string; jupyter_data: IOutput } = { | ||
type: 'output', | ||
id: nanoid(), |
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.
Not certain of the purpose of id
, but I suspect we actually want to lift this up.
667cb55
to
e278262
Compare
e278262
to
1f3a8c2
Compare
Output
into Outputs -> Output[]
Output
into Outputs -> Output[]
Output
into Outputs -> Output[]
Warning
Please do not review this PR. It's a proof-of-concept to illustrate conversation.
This PR is an alternative to #1661 that attempts to solve #1026. It does the following:
Tip
Internal AST is the AST that our transforms (and plugins) operate upon.
External AST is the AST that we publish to e.g. the web.
The factoring of our AST into two "branches" means that we can defer breaking changes to an arbitrary point in the future.
New Internal AST Form
The internal AST has the following form:
New External AST Form
In this PR, I don't break our published AST at all by storing a "replacement" AST as JSON in a node field.
Note
The pattern of
_future_ast
replacing the current node seems like it might be quite useful. A way to systematically define forwards-compatible AST.Closes #1674