Add support for semicolon output suppression #3806
Conversation
![devin-ai-integration[bot]](https://avatars.githubusercontent.com/in/811515?s=60&v=4){kind=small}
Co-Authored-By: Myles Scolnick <myles@marimo.io>
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
for more information, see https://pre-commit.ci
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
marimo/_ast/compiler.py
Outdated
| expr.end_col_offset = final_expr.end_col_offset | ||
| expr = ast.Expression(const) | ||
| # Creating an expression clears source info, so it needs to be set back | ||
| expr.lineno = getattr(const, "lineno", 0) |
There was a problem hiding this comment.
This should not be indented
| return await EXECUTION_TYPES[execution_type].execute_cell_async( | ||
| cell, glbls, graph | ||
| ) | ||
| executor = EXECUTION_TYPES[execution_type]() |
There was a problem hiding this comment.
Oh I see. Yeah, it's reasonable to put the change here.
But just setting last expr to None seems ideal. More execution types will require more churn and meta data ferrying.
Co-Authored-By: Myles Scolnick <myles@marimo.io>
Co-Authored-By: Myles Scolnick <myles@marimo.io>
| [ | ||
| exec_req.get( | ||
| """ | ||
| x = 3; # comment |
There was a problem hiding this comment.
Tests don't actually capture the intended behavior.
3;But assignment expression would return anything anyway
| result = eval(cell.last_expr, glbls) | ||
| if cell.suppress_output: | ||
| return None | ||
| return result if result is not None else None |
| lines = code.strip().split("\n") | ||
| for line in reversed(lines): | ||
| line = line.split("#")[0].strip() | ||
| if line: | ||
| return line.endswith(";") | ||
| return False |
There was a problem hiding this comment.
| lines = code.strip().split("\n") | |
| for line in reversed(lines): | |
| line = line.split("#")[0].strip() | |
| if line: | |
| return line.endswith(";") | |
| return False | |
| tokens = tokenize(io.BytesIO(code.strip().encode("utf-8").readline) | |
| for token in reversed(list(tokens)): | |
| if token.type in (token_types.ENDMARKER, token_types.NEWLINE, token_types.COMMENT): | |
| continue | |
| return token.string == ";" | |
| return False |
| @@ -226,6 +245,7 @@ def compile_cell( | |||
| body=body, | |||
| last_expr=last_expr, | |||
| cell_id=cell_id, | |||
| suppress_output=ends_with_semicolon(code), | |||
There was a problem hiding this comment.
| suppress_output=ends_with_semicolon(code), |
| @@ -141,18 +161,17 @@ def compile_cell( | |||
| final_expr = module.body[-1] | |||
| if isinstance(final_expr, ast.Expr): | |||
There was a problem hiding this comment.
| if isinstance(final_expr, ast.Expr): | |
| # Supress expression if ends in semi colon | |
| if isinstance(final_expr, ast.Expr) and not ends_with_semicolon(code): |
| setattr(const, "col_offset", final_expr.end_col_offset) | ||
| setattr(const, "end_col_offset", final_expr.end_col_offset) | ||
| setattr(const, "lineno", len(code.splitlines()) + 1) | ||
| expr = ast.Expression(const) | ||
| # use code over body since lineno corresponds to source | ||
| const.lineno = len(code.splitlines()) + 1 | ||
| expr.lineno = const.lineno | ||
| # Creating an expression clears source info, so it needs to be set back | ||
| expr.col_offset = final_expr.end_col_offset | ||
| expr.end_col_offset = final_expr.end_col_offset | ||
| # Creating an expression clears source info, so it needs to be set back | ||
| setattr(expr, "lineno", getattr(const, "lineno", 0)) | ||
| setattr(expr, "col_offset", final_expr.end_col_offset) | ||
| setattr(expr, "end_col_offset", final_expr.end_col_offset) |
There was a problem hiding this comment.
Undo the changes here, still a bad block
| return await EXECUTION_TYPES[execution_type].execute_cell_async( | ||
| cell, glbls, graph | ||
| ) | ||
| executor = EXECUTION_TYPES[execution_type]() |
There was a problem hiding this comment.
Would revert changes for full file
| @@ -0,0 +1,67 @@ | |||
| from tests.conftest import ExecReqProvider, MockedKernel | |||
There was a problem hiding this comment.
Would run tests over in tests/_ast/compiler.py (since no longer a executor change)
Try
11;expression = 1;1;2;3;41;2;3;4;1; # Has a comment1 # Has a comment;mo.md("# splits on # are less than ideal") # Contains a #mo.md("# splits on # are less than ideal"); # Contains a #and ensure expr is const None in the ";" cases
| # Whether to suppress output for this cell | ||
| suppress_output: bool = False |
There was a problem hiding this comment.
| # Whether to suppress output for this cell | |
| suppress_output: bool = False |
|
Devin is currently unreachable - the session may have died. |
2 similar comments
|
Devin is currently unreachable - the session may have died. |
|
Devin is currently unreachable - the session may have died. |
|
Devin is currently unreachable - the session may have died. |
3 similar comments
|
Devin is currently unreachable - the session may have died. |
|
Devin is currently unreachable - the session may have died. |
|
Devin is currently unreachable - the session may have died. |
|
@dmadisetti thanks for reviewing it. usually i don't try to give it PR comments (although you totally could). if it gets close enough on the first try I'll use it, otherwise I just close the PR and do it myself or build on top of it. |
|
Closed for #3830 |
## 📝 Summary Replaces #3806 ## 🔍 Description of Changes Resolves #3726 @mscolnick --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Implements #3726
Link to Devin run: https://app.devin.ai/sessions/0eeb86c476d34187b3e2f2cb80edc3d6