Skip to content

Commit 5a29e2d

Browse files
fix: filter sql defs from generated return (#6805)
## 📝 Summary Previously defining a sql variable would also add it to the return signature. This is incorrect, since one would actually get a name error. Added a test to prevent future regressions. --------- Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
1 parent 4ea8cf3 commit 5a29e2d

File tree

9 files changed

+65
-28
lines changed

9 files changed

+65
-28
lines changed

examples/sql/histograms.py

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@
1212

1313
import marimo
1414

15-
__generated_with = "0.15.5"
15+
__generated_with = "0.17.0"
1616
app = marimo.App(width="medium")
1717

1818

@@ -60,7 +60,7 @@ def _(URL, mo):
6060
FROM dataset
6161
"""
6262
)
63-
return (dataset,)
63+
return
6464

6565

6666
@app.cell

examples/sql/misc/electric_vehicles.py

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

1212
import marimo
1313

14-
__generated_with = "0.16.0"
14+
__generated_with = "0.17.0"
1515
app = marimo.App(width="medium")
1616

1717

@@ -36,7 +36,7 @@ def _(mo):
3636
select * from evs
3737
"""
3838
)
39-
return (evs,)
39+
return
4040

4141

4242
@app.cell

examples/sql/misc/sql_cars.py

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1,18 +1,18 @@
11
# /// script
22
# requires-python = ">=3.9"
33
# dependencies = [
4-
# "altair==5.4.1",
5-
# "duckdb==1.1.1",
4+
# "altair>=5.4.1",
5+
# "duckdb>=1.1.1",
66
# "marimo",
7-
# "polars==1.18.0",
8-
# "pyarrow==18.1.0",
9-
# "vega-datasets==0.9.0",
7+
# "polars>=1.18.0",
8+
# "pyarrow>=18.1.0",
9+
# "vega-datasets>=0.9.0",
1010
# ]
1111
# ///
1212

1313
import marimo
1414

15-
__generated_with = "0.16.0"
15+
__generated_with = "0.17.0"
1616
app = marimo.App(width="medium")
1717

1818

@@ -31,7 +31,7 @@ def _(cars_df, mo):
3131
CREATE OR REPLACE TABLE cars AS SELECT * FROM cars_df;
3232
"""
3333
)
34-
return (cars,)
34+
return
3535

3636

3737
@app.cell
@@ -54,7 +54,7 @@ def _(origin):
5454
@app.cell
5555
def _(mo, origin, top_n):
5656
mo.md(
57-
f"""##Top {top_n.value} Cars {f"in {origin.value}" if origin.value != None else ""} """
57+
f"""##Top {top_n.value} Cars {f"in {origin.value}" if origin.value != None else ""}"""
5858
)
5959
return
6060

examples/sql/read_csv.py

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

1212
import marimo
1313

14-
__generated_with = "0.16.0"
14+
__generated_with = "0.17.0"
1515
app = marimo.App(width="medium")
1616

1717

@@ -118,7 +118,7 @@ def _(mo):
118118
CREATE TABLE myTable AS SELECT * FROM "data.csv"
119119
"""
120120
)
121-
return (mytable,)
121+
return
122122

123123

124124
@app.cell

examples/sql/read_json.py

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

1212
import marimo
1313

14-
__generated_with = "0.16.0"
14+
__generated_with = "0.17.0"
1515
app = marimo.App(width="medium")
1616

1717

@@ -128,7 +128,7 @@ def _(mo):
128128
CREATE OR REPLACE TABLE myTable AS SELECT * FROM 'data.json'
129129
"""
130130
)
131-
return (mytable,)
131+
return
132132

133133

134134
@app.cell

examples/sql/read_parquet.py

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

1212
import marimo
1313

14-
__generated_with = "0.16.0"
14+
__generated_with = "0.17.0"
1515
app = marimo.App(width="medium")
1616

1717

@@ -117,7 +117,7 @@ def _(mo):
117117
CREATE OR REPLACE TABLE myTable AS SELECT * FROM 'data.parquet'
118118
"""
119119
)
120-
return (mytable,)
120+
return
121121

122122

123123
@app.cell

examples/ui/table.py

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import marimo
22

3-
__generated_with = "0.16.0"
3+
__generated_with = "0.17.0"
44
app = marimo.App()
55

66

@@ -43,14 +43,15 @@ def cell_hover(row_id: str, column_name: str, value) -> str:
4343
hover_template=cell_hover,
4444
)
4545
hover_table
46-
return (hover_table,)
46+
return
4747

4848

4949
@app.cell
5050
def _(table):
5151
table.value
5252
return
5353

54+
5455
@app.cell
5556
def _(mo):
5657
# Demonstrate a long table with a sticky header and a custom max height
@@ -61,8 +62,7 @@ def _(mo):
6162
max_height=300,
6263
)
6364
long_table
64-
return (long_table,)
65-
65+
return
6666

6767

6868
if __name__ == "__main__":

marimo/_ast/codegen.py

Lines changed: 15 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -236,15 +236,24 @@ def to_functiondef(
236236
# other static analysis tools can capture unused variables across cells.
237237
defs: tuple[str, ...] = tuple()
238238
if cell.defs:
239+
# SQL defs should not be included in the return value.
240+
sql_defs = (
241+
{
242+
name
243+
for name, value in variable_data.items()
244+
if value.language == "sql"
245+
}
246+
if variable_data
247+
else set()
248+
)
239249
# There are possible name error cases where a cell defines, and also
240250
# requires a variable. We remove defs from the signature such that
241251
# this causes a lint error in pyright.
242-
if used_refs is None:
243-
defs = tuple(name for name in sorted(cell.defs))
244-
else:
245-
defs = tuple(
246-
name for name in sorted(cell.defs) if name in used_refs
247-
)
252+
defs = tuple(
253+
name for name in sorted(cell.defs) if name not in sql_defs
254+
)
255+
if used_refs is not None:
256+
defs = tuple(name for name in defs if name in used_refs)
248257

249258
decorator = to_decorator(cell.config, fn=fn)
250259
prefix = "" if not cell.is_coroutine() else "async "

tests/_ast/test_codegen.py

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -662,6 +662,34 @@ def test_dotted_names_filtered_from_signature(self) -> None:
662662
assert "my_schema.pokemon_db" not in fndef
663663
assert "def foo(my_schema.pokemon_db" not in fndef
664664

665+
def test_sql_defs_filtered_from_return(self) -> None:
666+
"""Test that SQL definitions are filtered from return but can still be referenced."""
667+
668+
# Cell 1: defines a SQL variable (cars) - should NOT be in return
669+
code1 = "empty = mo.sql('CREATE TABLE cars_df ();')"
670+
# Cell 2: uses the SQL variable (cars) - should appear in signature
671+
code2 = "result = cars_df.filter(lambda x: x > 0); empty"
672+
expected = wrap_generate_filecontents(
673+
[code1, code2], ["cell1", "cell2"]
674+
)
675+
assert (
676+
"\n".join(
677+
[
678+
"@app.cell",
679+
"def cell1(mo):",
680+
" empty = mo.sql('CREATE TABLE cars_df ();')",
681+
" return (empty,)", # Doesn't return cars_df
682+
"",
683+
"",
684+
"@app.cell",
685+
"def cell2(cars_df, empty):",
686+
" result = cars_df.filter(lambda x: x > 0); empty",
687+
" return",
688+
]
689+
)
690+
in expected
691+
)
692+
665693
def test_should_remove_defaults(self) -> None:
666694
code = "x = 0"
667695
cell = compile_cell(code)

0 commit comments

Comments
 (0)