fix: correct awkward backend field-leak, behavior, and dict-input bugs#718
Draft
henryiii wants to merge 1 commit into
Draft
fix: correct awkward backend field-leak, behavior, and dict-input bugs#718henryiii wants to merge 1 commit into
henryiii wants to merge 1 commit into
Conversation
Fixes several bugs in the Awkward backend: - _wrap_result leaked stale px/py coordinate fields, since the per-branch exclusion tuples omitted the px/py momentum aliases. Replace the five hand-copied tuples with shared, geometry-tiered frozensets that include all momentum aliases. - Record methods lost their behavior: rebuilding scalar results via ak.Array dropped the behavior dict, so chaining two methods on an ak.Record raised AttributeError when vector was not globally registered. Preserve the input record's behavior on rebuild. - vector.Array(dict) crashed with AttributeError; now dict-of-columns inputs are converted to ak.Array like lists/ndarrays. - Replace __builtins__["zip"] (a CPython dict implementation detail) with builtins.zip. - Remove a dead behavior-mutation loop in vector.Array whose effect was unconditionally overwritten by the final with_name(..., behavior=...). - Fix copy-pasted docstrings (TemporalAwkward, VectorAwkward4D, Momentum cross-references, MomentumArray2D.allclose). Assisted-by: ClaudeCode:claude-opus-4-8
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #718 +/- ##
==========================================
+ Coverage 87.09% 87.13% +0.03%
==========================================
Files 96 96
Lines 11231 11229 -2
==========================================
+ Hits 9782 9784 +2
+ Misses 1449 1445 -4 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
43 tasks
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.
🤖 AI text below 🤖
Part of #711.
Fixes several bugs in the Awkward backend (each verified before/after).
1.
_wrap_resultleaked stalepx/pyfields (HIGH)The five per-branch "extra field" exclusion tuples in
_wrap_resultomitted thepx/pymomentum aliases, so the pre-computationpx/pywere carried into the output alongside the freshly computedx/y.Fixed structurally: the five hand-copied tuples are replaced with shared, geometry-tiered module-level frozensets (
_azimuthal_fields,_longitudinal_fields,_temporal_fieldsand their unions) that include all generic and momentum-alias coordinate names. The tiering is preserved per branch (a pure-azimuthal return still carries longitudinal/temporal coordinates as extras).2. Record methods lost behavior (MEDIUM)
When inputs are
ak.Records/scalars,_wrap_resultrebuilds length-1 arrays viaak.Array(...), which dropped the behavior dict. Without global registration the output was a behavior-lessRecord, so chaining a second method raisedAttributeError.Fixed by capturing the input record's behavior and passing it to the
ak.Array(...)rebuilds. Regression test runs in a subprocess so global registration state cannot leak in.3.
vector.Array(dict)crashed (MEDIUM)The docstring promises all
ak.Arraysignatures, but onlylist/ndarrayargs were converted; a dict-of-columns raisedAttributeError: 'dict' object has no attribute 'type'.Fixed by also converting
dictinputs viaawkward.Array(...)(dask arrays still pass through unchanged).4.
__builtins__["zip"](LOW)dict(__builtins__["zip"](...))relied on__builtins__being a dict (a CPython implementation detail). Replaced withimport builtins+builtins.zip(...).5. Dead behavior-mutation loop (LOW)
The loop in
vector.Arraythat set.behavioron temporary projected column arrays had no observable effect, since the finalawkward.with_name(..., behavior=...)unconditionally replaces the output behavior. Removed.Docstring fixes
Corrected copy-pasted docstrings:
TemporalAwkward("longitudinal" -> "temporal"),VectorAwkward4D(described itself as a momentum class),MomentumAwkward2D/3D/4Dcross-references (Vector/Momentum swapped), andMomentumArray2D.allclose("MomentumArray4D" -> "MomentumArray2D").Tests
Added regression tests to
tests/backends/test_awkward.pycovering all five fixes. Full suite (--ignore=tests/test_notebooks.py): 817 passed, 3 skipped (dask_awkward/optree/jax not installed). Doctests andprek -apass.🤖 Generated with Claude Code