Skip to content

Fix bundle() on Python 3.13 by snapshotting the caller's locals - #54

Open
Max Freedom Pollard (MaxFreedomPollard) wants to merge 1 commit into
microsoft:mainfrom
MaxFreedomPollard:fix/python-3.13-frame-locals
Open

Fix bundle() on Python 3.13 by snapshotting the caller's locals#54
Max Freedom Pollard (MaxFreedomPollard) wants to merge 1 commit into
microsoft:mainfrom
MaxFreedomPollard:fix/python-3.13-frame-locals

Conversation

@MaxFreedomPollard

Copy link
Copy Markdown

Fixes #43.

Root cause

bundle() captures the calling frame's locals so a trainable operator can be re-executed with them:

prev_f_locals = inspect.stack()[1].frame.f_locals

Python 3.13 implemented PEP 667, which changed frame.f_locals to return a live FrameLocalsProxy instead of a dict. FunModule.__init__ then rejects it:

AssertionError: _ldict must be a dictionary. or None

Every bundle() call fails on 3.13 as a result, so the package is effectively unusable there — which is what #43 reports. Ching-An Cheng (@chinganc) identified this cause in the issue thread; this PR is the fix for it.

The fix

prev_f_locals = dict(inspect.stack()[1].frame.f_locals)

I went with a snapshot at the capture site rather than relaxing the isinstance(_ldict, dict) check, for two reasons:

  • It restores the pre-3.13 behaviour exactly. Before 3.13, f_locals handed back a point-in-time dict copy. FrameLocalsProxy is a live view of the frame, so merely accepting it would leave 3.13 with subtly different semantics from every other version.
  • Nothing else has to change. FunModule still receives a real dict, so the existing assertion and the _ldict.copy() on the next line keep working untouched.

FrameLocalsProxy is a Mapping, so dict(...) is the documented way to materialise it, and on ≤3.12 this is just a copy of a dict that was about to be copied anyway.

I also checked the other frame manipulation in bundle.py for 3.13 breakage — update_local() calls ctypes.pythonapi.PyFrame_LocalsToFast, which I expected to be gone in 3.13. It is still exported and still works, and the overwrite_python_recursion path it serves passes on 3.13, so nothing further is needed.

Coverage

tests/unit_tests/test_bundle.py already exercised the broken path — it applies @bundle() inside a function, which is exactly what triggers the capture — it just never ran on 3.13. So this PR:

  • adds an explicit assertion that the captured locals are a dict containing the caller's variables, which pins the behaviour rather than testing it incidentally
  • extends the CI matrix to ["3.9", "3.13"] with fail-fast: false, so 3.13 is actually covered from now on

Without the fix, test_bundle.py fails on 3.13 with the AssertionError above and passes on 3.9. With it, both pass.

Verification

3.9 3.12 3.13
python tests/unit_tests/run.py 22/22, exit 0 22/22, exit 0 22/22, exit 0

The matrixed workflow, run on this exact commit in my fork: https://github.com/MaxFreedomPollard/Trace/actions/runs/33941552555 — both legs green, 22/22 each.

ruff==0.0.261 reports the same 60 pre-existing findings before and after this change, and black==23.3.0 proposes no changes to any added line (both files already differ from black on main, so I left the rest alone). No API, signature, or dependency changes.

Fixes microsoft#43.

bundle() captures the calling frame's locals so that a trainable operator
can be re-executed with them. Python 3.13 changed frame.f_locals to return
a live FrameLocalsProxy rather than a dict (PEP 667), so the captured value
is no longer a dict and FunModule's isinstance check rejects it:

    AssertionError: _ldict must be a dictionary. or None

Every bundle() call fails on 3.13 as a result, which is what microsoft#43 reports.

Taking dict(...) of the proxy at the capture site restores the pre-3.13
behaviour exactly -- a plain dict snapshot of the caller's locals -- on
every supported version, and needs no change to the isinstance check.

tests/unit_tests/test_bundle.py already exercises the failing path, since
it applies @Bundle inside a function; it simply never ran on 3.13. This
adds an explicit assertion that the captured locals are a dict holding the
caller's variables, and extends the CI matrix to 3.9 and 3.13 so the
regression is covered from now on.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Trace doesn't work with python 3.13

1 participant