Skip to content
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

raise a NameError during function definition, same as during cell execution #3957

Open
alefminus opened this issue Mar 2, 2025 · 5 comments
Labels
enhancement New feature or request

Comments

@alefminus
Copy link

alefminus commented Mar 2, 2025

Description

Currently there is an asymmetry between writing code in a cell and creating a function in a cell and later invoking it, in that the former will raise a NameError on an undefined variable immediately.

This seems to me to be because marimo delegates the checking to python, as indicated by the stack trace showing the running of the marimo produced code.

However, from a usage point of view, this is not symmetric:

  • if I write a cell "a + not_defined" and invoke it I immediately get feedback "NameError: not_defined is not defined".
  • if I write a cell with "lambda a: a + not_defined" I will however not get any error until a later time, when I invoke it, probably from another cell.

Thanks for marimo!

Alon

Suggested solution

I'm not proposing how to fix it, although I guess it would require compiling and inspection, so not trivial, but I have searched and could not find any prior request, so I'm filing this issue.

Alternative

No response

Additional context

import marimo

__generated_with = "0.11.13"
app = marimo.App(width="medium")


@app.cell
def _(not_defined):
    a = lambda x: x + not_defined
    return (a,)


@app.cell
def _(a):
    a(10)
    return


@app.cell
def _(also_not_defined):
    def f(x):
        return x + also_not_defined
    return (f,)


@app.cell
def _(f):
    f(10)
    return


@app.cell
def _():
    x = 20
    return (x,)


@app.cell
def _(not_defined, x):
    x + not_defined
    return


if __name__ == "__main__":
    app.run()
@alefminus alefminus added the enhancement New feature or request label Mar 2, 2025
@dmadisetti
Copy link
Collaborator

dmadisetti commented Mar 3, 2025

This is because signatures don't actually do anything. If you tried to save this in marimo, it would be overwritten since the arguments are just there to show dependency, and don't have any direct functionality. I think this is fine, since variables like not_defined are not known by the graph, and as such cannot be meaningfully introduced into a cell for reactive execution.

Stay tuned for top level functions which will work as you expect: https://github.com/marimo-team/meps/blob/mep-0007/mep-0008.md#top-level-api-discussions

edit: see below

@dmadisetti dmadisetti reopened this Mar 3, 2025
@dmadisetti
Copy link
Collaborator

Woops, I clicked the wrong thing.

I actually misunderstood your issue. Yes! There is "strict mode" that will create an error if there are undefined variables on execution. This is currently undocumented since there are performance penalties, but it's useful for internal testing- and maybe something to revisit. If you wanted to try it:

[experimental]
execution_type = "strict"

@alefminus
Copy link
Author

cool, did not know of that! works, but unfortunately it breaks sqlalchemy, and it appears it does it because the implementation (like you said) includes a deep copy of the sqlalchemy connection type which does not support it:

This cell raised an exception: UnknownError('UnknownError(msg='Could not clone reference `con` of type sqlalchemy.engine.base. Connection try wrapping the object in a `zero_copy` call. If this is a common object type, consider making an issue on the marimo GitHub repository to never deepcopy.', type='unknown')')

Trying to fix that I get a new error (wrapped con with marimo._runtime.copy.zero_copy)

This cell raised an exception: ValueError(''__firstlineno__' in __slots__ conflicts with class variable')

From the console I see that sqlite requires same thread invocation and that requirement is broken:

Exception during reset or similar
Traceback (most recent call last):
  File "/home/alon/.cache/uv/archive-v0/EK09kVJH_JizlesDbke3f/lib64/python3.13/site-packages/sqlalchemy/pool/base.py", line 986, in _finalize_fairy
    fairy._reset(
    ~~~~~~~~~~~~^
        pool,
        ^^^^^
    ...<2 lines>...
        asyncio_safe=can_manipulate_connection,
        ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
    )
    ^
  File "/home/alon/.cache/uv/archive-v0/EK09kVJH_JizlesDbke3f/lib64/python3.13/site-packages/sqlalchemy/pool/base.py", line 1432, in _reset
    pool._dialect.do_rollback(self)
    ~~~~~~~~~~~~~~~~~~~~~~~~~^^^^^^
  File "/home/alon/.cache/uv/archive-v0/EK09kVJH_JizlesDbke3f/lib64/python3.13/site-packages/sqlalchemy/engine/default.py", line 700, in do_rollback
    dbapi_connection.rollback()
    ~~~~~~~~~~~~~~~~~~~~~~~~~^^
sqlite3.ProgrammingError: SQLite objects created in a thread can only be used in that same thread. The object was created in thread id 139813733541696 and this is thread id 139813423535808.

To reproduce:

#!/bin/bash

cat > .marimo.toml <<EOF
[experimental]
execution_type = "strict"
EOF
cat > strict_sqlalchemy.py <<EOF
import marimo

__generated_with = "0.11.13"
app = marimo.App(width="medium")


@app.cell
def _():
    from sqlalchemy import create_engine, text
    return (create_engine, text)


@app.cell
def _(create_engine, text):
    engine = create_engine('sqlite://')
    con = engine.connect()
    return (con,)


@app.cell
def _(con, text):
    list(con.execute(text('select 1+2+3')))


if __name__ == '__main__':
    app.main()
EOF

uvx --with sqlalchemy marimo edit strict_sqlalchemy.py

Note that I had to change one line in the currently released version, marimo._sql.engines line 174, query: str, in case the db is sqlite remained undefined, so I changed it to "query: str = None"

@dmadisetti
Copy link
Collaborator

Yeah, I think zero_copy might be a little over engineered. I think it might be fair to provide the unwrapped value to the runtime opposed to shadowing the variable attributes.

But thanks for trying out the feature. I've used it for debugging but had to turn it off long term. Do you think something like strict mode might be useful for general consumption?

@alefminus
Copy link
Author

alefminus commented Mar 3, 2025 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants