marimo 0.6.14 no longer imports xdsl without error

[superlopuh]

Describe the bug

After updating marimo, our framework is no longer loaded correctly. I'm not sure exactly what caused this change by looking at the changelog, but this works fine in 0.6.13 and no longer works in 0.6.14:

from xdsl.dialects.riscv import Add

Here's an online notebook with the issue. https://marimo.app/l/6jest0

We do some messy things with __annotations__ etc of a class, I'm not sure how that interacts with whatever magic Marimo does for code execution, but I don't see anything that could have affected it specifically in 0.6.14, unless it's one of the dependencies.

Environment

{ "marimo": "0.6.23", "OS": "Darwin", "OS Version": "23.5.0", "Processor": "arm", "Python Version": "3.12.3", "Binaries": { "Browser": "126.0.6478.127", "Node": "v21.7.1" }, "Requirements": { "click": "8.1.7", "importlib-resources": "6.4.0", "jedi": "0.19.1", "markdown": "3.6", "pymdown-extensions": "10.8.1", "pygments": "2.18.0", "tomlkit": "0.12.5", "uvicorn": "0.30.1", "starlette": "0.37.2", "websockets": "12.0", "typing-extensions": "4.12.2", "black": "24.4.2" } }

Code to reproduce

from xdsl.dialects.riscv import Add

[superlopuh]

Just in case it helps debug something, the error is there because the subclass has in its annotations, the __annotations__ of a superclass, while not having the corresponding field in its __dict__, and the code that checks for consistencies for each of these fails when traversing the mro()

[superlopuh]

I mistook the failure to import as a minimal reproducible example of my bug, but that's not quite correct. I have an issue with a local notebook, and I'm still trying to figure out exactly what the error is, but it's not the one I described above. I'll close this issue for now, sorry about the noise.

[superlopuh]

Ok I've found a way to reproduce this online: when I import from our framework in multiple places, it gives weird errors, that are due to the __annotations__ being mixed up. I'm not sure whether we're doing something wrong in xdsl itself, but the error is not visible when running in any environment other than marimo 0.6.14 and up. Here's a notebook with the behaviour:

https://marimo.app/l/kf3rz4

[akshayka]

@superlopuh , thanks for opening the issue. I took a look at the commits in 0.6.14 and couldn't figure out what could have caused this :/

[superlopuh]

I wonder if it's one of the dependencies updated in the release? https://github.com/marimo-team/marimo/commit/b0553296c491fe680c121e195d09df5bdf6c52d1 The Pyodide update seems like the most likely cause, maybe there was a change in the import functionality in the major version bump.

[akshayka]

I don't think it's related to Pyodide. I can reproduce your issue locally (and your code works in the Pyodide REPL).

From your example, importing Add first, then Attribute works; but doing the imports in the opposite order fails.

I think this is a bug in marimo; I'm just not sure what specifically about your package causes it to surface.

[akshayka]

This is the "weird error" I see:

@superlopuh , you mentioned that __annotations__ get mixed up. Can you give any more insight or clues into what might be going wrong? I'd like to try and fix this for you.

[akshayka]

I think I've narrowed down the issue. We inspect attributes of variables in memory after running a cell (in an attempt to find dataframes), and it looks like xdsl doesn't like that.

[akshayka]

Calling the following function on xdsl.ir.Attribute ends up breaking subsequent xdsl imports:

https://github.com/marimo-team/marimo/blob/main/marimo/_plugins/ui/_impl/tables/types.py#L16-L25

In particular, it's the isinstance check against the runtime_checkable protocol that breaks things. Any idea why that would be? (Edit: could be related to this CPython issue, which was supposed to be resolved in Python 3.12+ -- however, I can repro the xdsl issue in marimo using Python 3.12)

Two options for fixing this:

1. Maybe this is a bug on your end? I don't know enough about how xdsl is implemented to know. It does seem surprising that an isinstance check interferes with imports.
2. We could be more explicit in checking for dataframe-ness of an object, by just doing a bunch of isinstance checks against known types, without using runtime_checkable

[superlopuh]

Am I understanding correctly that removing this check makes the error go away? It does seem similar to the issue you linked. It's always possible that the bug is on our end, but it's curious that it's only ever reproducable in this marimo scenario. If runtime_checkable is known to have issues with supported versions of Python, then maybe it would be worth trying a different way to perform the check?

[akshayka]

It must just be because runtime checkable has side-effects. We'll use a different check on our side -- no need to change your library code. Will let you know when a fix is released.

[superlopuh]

Amazing, thank you!