fix: enum comparisons in module reloading#9862
Draft
mscolnick wants to merge 1 commit into
Draft
Conversation
When a module containing an enum.Enum was reloaded, dependent modules that had already imported the enum (e.g. via from enums import Fruits) kept references to the old enum class, while re-imported symbols pointed at the new class. Enum members from different class objects do not compare equal, so functions like is_orange(Fruits.ORANGE) incorrectly returned False after reload even when the enum definition was unchanged. superreload now restores updated old objects into the reloaded module's namespace after patching, so dependent modules and re-imports continue to share the same class/function instances.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Contributor
There was a problem hiding this comment.
1 issue found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="marimo/_runtime/reload/autoreload.py">
<violation number="1" location="marimo/_runtime/reload/autoreload.py:573">
P1: The new namespace rebinding restores stale objects even when `update_generic` cannot update them, so unsupported module attributes can be reverted to pre-reload state.</violation>
</file>
Architecture diagram
sequenceDiagram
participant User as User Kernel
participant Reloader as ModuleReloader
participant SysMod as sys.modules
participant DepMod as Dependent Module
participant EnumMod as Enum Module
Note over User,EnumMod: Runtime Module Reload Flow
User->>Reloader: check(modules, reload=True)
Reloader->>Reloader: superreload() begins
loop For each changed name in enum module
Reloader->>SysMod: Get old object references
SysMod-->>Reloader: old_refs (weak references)
Reloader->>Reloader: update_generic(old_obj, new_obj)
Note over Reloader: Patch old object to match new,<br/>preserving object identity
Reloader->>EnumMod: Keep updated old object in __dict__
Note over Reloader,EnumMod: NEW: Overwrite module namespace<br/>so that imported names point to<br/>the patched old instance
EnumMod-->>EnumMod: module.__dict__[name] = old_obj
end
Note over DepMod: Dependent module already imported<br/>via `from enums import Fruits`
DepMod-->>DepMod: Fruits refers to patched old class
EnumMod-->>EnumMod: Fruits also refers to same patched old class
User->>DepMod: call is_orange(Fruits.ORANGE)
DepMod->>DepMod: fruit == Fruits.ORANGE
alt Same object identity
Note over DepMod: Both Fruits classes are<br/>the same patched instance
DepMod-->>User: True (correct)
else Different objects (without fix)
Note over DepMod: Fruits.ORANGE from different<br/>class objects don't compare equal
DepMod-->>User: False (bug)
end
Note over DepMod,EnumMod: Both modules now share same<br/>class/function instances after reload
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| # Keep the updated old object in the module namespace so that | ||
| # dependent modules (e.g. `from enums import Fruits`) still refer | ||
| # to the same class/function instances after reload. | ||
| if kept_old_obj is not None: |
Contributor
There was a problem hiding this comment.
P1: The new namespace rebinding restores stale objects even when update_generic cannot update them, so unsupported module attributes can be reverted to pre-reload state.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At marimo/_runtime/reload/autoreload.py, line 573:
<comment>The new namespace rebinding restores stale objects even when `update_generic` cannot update them, so unsupported module attributes can be reverted to pre-reload state.</comment>
<file context>
@@ -558,12 +558,20 @@ def superreload(
+ # Keep the updated old object in the module namespace so that
+ # dependent modules (e.g. `from enums import Fruits`) still refer
+ # to the same class/function instances after reload.
+ if kept_old_obj is not None:
+ module.__dict__[name] = kept_old_obj
</file context>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #9808
When a module containing an enum.Enum was reloaded, dependent modules that had already imported the enum (e.g. via from enums import Fruits) kept references to the old enum class, while re-imported symbols pointed at the new class. Enum members from different class objects do not compare equal, so functions like is_orange(Fruits.ORANGE) incorrectly returned False after reload even when the enum definition was unchanged.
superreload now restores updated old objects into the reloaded module's namespace after patching, so dependent modules and re-imports continue to share the same class/function instances.