Skip to content

Commit 04733eb

Browse files
committed
Handle parsing errors
1 parent e84f4bb commit 04733eb

File tree

8 files changed

+83
-112
lines changed

8 files changed

+83
-112
lines changed

foo.py

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
# /// script
2+
# requires-python = ">=3.13"
3+
# dependencies = []
4+
# ///
5+
6+
7+
def main() -> None:
8+
print("Hello from foo.py!")
9+
10+
11+
if __name__ == "__main__":
12+
main()

marimo/_ast/load.py

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
from marimo._ast.app import App, InternalApp
1111
from marimo._ast.parse import (
1212
MarimoFileError,
13+
is_non_marimo_python_script,
1314
parse_notebook,
1415
)
1516
from marimo._schemas.serialization import NotebookSerialization, UnparsableCell
@@ -67,9 +68,16 @@ def _static_load(filepath: Path) -> Optional[App]:
6768
contents = _maybe_contents(filepath)
6869
if not contents:
6970
return None
71+
7072
notebook = parse_notebook(contents)
73+
74+
if notebook and is_non_marimo_python_script(notebook):
75+
# Should fail instead of overriding contents
76+
raise MarimoFileError(f"Python script {filepath} is not a marimo notebook.")
77+
7178
if notebook is None or not notebook.valid:
7279
return None
80+
7381
app = App(**notebook.app.options, _filename=str(filepath))
7482
for cell in notebook.cells:
7583
if isinstance(cell, UnparsableCell):
@@ -90,7 +98,6 @@ def get_notebook_status(filename: str) -> NotebookStatus:
9098
9199
Raises:
92100
SyntaxError: If the file contains a syntax error
93-
UnknownPythonScriptError: If the file is an unrecognized Python script format
94101
"""
95102
path = Path(filename)
96103

@@ -153,8 +160,7 @@ def load_app(filename: Optional[str]) -> Optional[App]:
153160
from marimo._convert.markdown.markdown import convert_from_md_to_app
154161

155162
return convert_from_md_to_app(contents) if contents else None
156-
157-
if not path.suffix == ".py":
163+
elif not path.suffix == ".py":
158164
raise MarimoFileError("File must end with .py or .md")
159165

160166
try:

marimo/_ast/parse.py

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -841,10 +841,10 @@ def parse_notebook(contents: str) -> Optional[NotebookSerialization]:
841841

842842
remaining = parser.extractor.contents[len(header.value) :]
843843
if len(remaining.strip()) != 0:
844+
# just a header is fine, anything else we would ignore and override
844845
violations.append(
845-
# TODO: Could be more specific if we have violation nums
846846
Violation(
847-
_unknown_content_violation_description,
847+
_non_marimo_python_script_violation_description,
848848
lineno=header.end_lineno + 2 if header.value else 1,
849849
)
850850
)
@@ -906,11 +906,13 @@ def parse_notebook(contents: str) -> Optional[NotebookSerialization]:
906906
)
907907

908908

909-
_unknown_content_violation_description = "Unknown content beyond header"
909+
_non_marimo_python_script_violation_description = (
910+
"non-marimo Python content beyond header"
911+
)
910912

911913

912-
def is_unknown_python_script(notebook: NotebookSerialization) -> bool:
914+
def is_non_marimo_python_script(notebook: NotebookSerialization) -> bool:
913915
return any(
914-
(v.description == _unknown_content_violation_description)
916+
(v.description == _non_marimo_python_script_violation_description)
915917
for v in notebook.violations
916918
)

marimo/_cli/cli.py

Lines changed: 15 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import click
1313

14+
from marimo._ast.parse import is_non_marimo_python_script
1415
import marimo._cli.cli_validators as validators
1516
from marimo import __version__, _loggers
1617
from marimo._ast import codegen
@@ -56,21 +57,8 @@ def helpful_usage_error(self: Any, file: Any = None) -> None:
5657

5758

5859
def check_app_correctness(filename: str) -> None:
59-
from marimo._ast.load import UnknownPythonScriptError
60-
6160
try:
6261
status = get_notebook_status(filename)
63-
except UnknownPythonScriptError:
64-
import os
65-
66-
stem = os.path.splitext(os.path.basename(filename))[0]
67-
raise click.ClickException(
68-
f"Unknown script - {filename} not recognized as a marimo notebook.\n\n"
69-
f" {green('Tip:')} Try converting with"
70-
"\n\n"
71-
f" marimo convert {filename} -o {stem}_nb.py\n\n"
72-
f" then open with marimo edit {stem}_nb.py"
73-
) from None
7462
except SyntaxError:
7563
import traceback
7664

@@ -83,6 +71,19 @@ def check_app_correctness(filename: str) -> None:
8371
click.echo(f"Failed to parse notebook: {filename}\n", err=True)
8472
raise click.ClickException(traceback.format_exc(limit=0)) from None
8573

74+
if status == "invalid" and filename.endswith(".py"):
75+
# fail for python scripts, almost certainly do not want to override contents
76+
import os
77+
78+
stem = os.path.splitext(os.path.basename(filename))[0]
79+
raise click.ClickException(
80+
f"Python script not recognized as a marimo notebook.\n\n"
81+
f" {green('Tip:')} Try converting with"
82+
"\n\n"
83+
f" marimo convert {filename} -o {stem}_nb.py\n\n"
84+
f" then open with marimo edit {stem}_nb.py"
85+
) from None
86+
8687
if status == "invalid":
8788
click.echo(
8889
green("tip")
@@ -99,6 +100,7 @@ def check_app_correctness(filename: str) -> None:
99100
default=False,
100101
abort=True,
101102
)
103+
102104
if status == "has_errors":
103105
# Provide a warning, but allow the user to open the notebook
104106
_loggers.marimo_logger().warning(

marimo/_cli/convert/commands.py

Lines changed: 18 additions & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -88,33 +88,24 @@ def convert(
8888
echo("File is already a valid marimo notebook.")
8989
return
9090

91-
# Check if it has the violation indicating it's an unknown Python script
92-
if parsed and any(
93-
v.description == "Unknown content beyond header"
94-
for v in parsed.violations
95-
):
96-
try:
97-
notebook = MarimoConvert.from_non_marimo_py_script(
98-
text
99-
).to_py()
100-
except ImportError as e:
101-
# Check if jupytext is the missing module in the cause chain
102-
if (
103-
e.__cause__
104-
and getattr(e.__cause__, "name", None) == "jupytext"
105-
):
106-
from marimo._cli.print import green
107-
108-
raise click.ClickException(
109-
f"{e}\n\n"
110-
f" {green('Tip:')} If you're using uv, run:\n\n"
111-
f" uvx --with=jupytext marimo convert {filename}"
112-
) from e
113-
raise
114-
else:
115-
# File has other issues (syntax errors, etc.)
116-
echo("File cannot be converted. It may have syntax errors.")
117-
return
91+
try:
92+
notebook = MarimoConvert.from_non_marimo_python_script(
93+
text
94+
).to_py()
95+
except ImportError as e:
96+
# Check if jupytext is the missing module in the cause chain
97+
if (
98+
e.__cause__
99+
and getattr(e.__cause__, "name", None) == "jupytext"
100+
):
101+
from marimo._cli.print import green
102+
103+
raise click.ClickException(
104+
f"{e}\n\n"
105+
f" {green('Tip:')} If you're using uv, run:\n\n"
106+
f" uvx --with=jupytext marimo convert {filename}"
107+
) from e
108+
raise
118109

119110
if output:
120111
output_path = Path(output)

marimo/_convert/converters.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ def from_py(source: str) -> MarimoConverterIntermediate:
5050
return MarimoConverterIntermediate(ir)
5151

5252
@staticmethod
53-
def from_non_marimo_py_script(source: str) -> MarimoConverterIntermediate:
53+
def from_non_marimo_python_script(
54+
source: str,
55+
) -> MarimoConverterIntermediate:
5456
"""Convert from a non-marimo Python script to marimo notebook.
5557
5658
This should only be used when the .py file is not already a valid
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
# /// script
2+
# requires-python = ">=3.12"
3+
# dependencies = [
4+
# "altair==5.4.1",
5+
# "duckdb==1.1.3",
6+
# "marimo",
7+
# ]
8+
# ///
9+
10+
def hello():
11+
print("hello, world")

tests/_ast/test_load.py

Lines changed: 8 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@
99

1010
from marimo import _loggers
1111
from marimo._ast import load
12+
from marimo._ast.parse import MarimoFileError
1213

1314
DIR_PATH = os.path.dirname(os.path.realpath(__file__))
1415

@@ -216,6 +217,13 @@ def test_get_codes_app_with_only_comments(load_app) -> None:
216217
app = load_app(get_filepath("test_app_with_only_comments"))
217218
assert app is None
218219

220+
@staticmethod
221+
def test_get_codes_non_marimo_python_script(static_load) -> None:
222+
with pytest.raises(MarimoFileError, match="is not a marimo notebook."):
223+
static_load(
224+
get_filepath("test_get_codes_non_marimo_python_script")
225+
)
226+
219227
@staticmethod
220228
def test_get_codes_app_with_no_cells(load_app) -> None:
221229
app = load_app(get_filepath("test_app_with_no_cells"))
@@ -310,66 +318,3 @@ def test_get_status_warn() -> None:
310318
)
311319
== "has_errors"
312320
)
313-
314-
@staticmethod
315-
def test_non_marimo_file_error(tmp_path) -> None:
316-
"""Test loading a marimo Python script raises UnknownPythonScriptError."""
317-
regular_python = textwrap.dedent(
318-
"""
319-
import numpy as np
320-
321-
def main():
322-
x = np.array([1, 2, 3])
323-
print(x)
324-
325-
if __name__ == "__main__":
326-
main()
327-
"""
328-
).strip()
329-
330-
filepath = tmp_path / "regular_script.py"
331-
filepath.write_text(regular_python)
332-
333-
with pytest.raises(
334-
load.UnknownPythonScriptError, match="is not a marimo notebook"
335-
):
336-
load.get_notebook_status(str(filepath))
337-
338-
@staticmethod
339-
def test_python_script_metadata_and_imports_error(tmp_path) -> None:
340-
"""Test loading a file with metadata and imports raises UnknownPythonScriptError."""
341-
metadata_only = textwrap.dedent(
342-
"""
343-
# /// script
344-
# dependencies = ["numpy", "pandas"]
345-
# requires-python = ">=3.8"
346-
# ///
347-
348-
import numpy as np
349-
"""
350-
).strip()
351-
352-
filepath = tmp_path / "imports_and_metadata_only.py"
353-
filepath.write_text(metadata_only)
354-
355-
with pytest.raises(
356-
load.UnknownPythonScriptError, match="is not a marimo notebook"
357-
):
358-
load.get_notebook_status(str(filepath))
359-
360-
@staticmethod
361-
def test_script_metadata_only_file(tmp_path) -> None:
362-
"""Test that files with only script metadata are allowed."""
363-
metadata_only = textwrap.dedent(
364-
"""
365-
# /// script
366-
# dependencies = ["numpy", "pandas"]
367-
# requires-python = ">=3.8"
368-
# ///
369-
"""
370-
).strip()
371-
372-
filepath = tmp_path / "metadata_only.py"
373-
filepath.write_text(metadata_only)
374-
375-
assert load.get_notebook_status(str(filepath)) == "invalid"

0 commit comments

Comments
 (0)