Skip to content

fix: interpretation-layer bugs (pandas NaN fallback, typename parsing, strided objects, bitset)#1660

Open
henryiii wants to merge 2 commits into
mainfrom
fix-interpretation-misc
Open

fix: interpretation-layer bugs (pandas NaN fallback, typename parsing, strided objects, bitset)#1660
henryiii wants to merge 2 commits into
mainfrom
fix-interpretation-misc

Conversation

@henryiii

Copy link
Copy Markdown
Member

🤖 AI text below 🤖

Part of #1646.

Fixes a batch of verified bugs in the interpretation layer, found during review.

Fixed

  1. library.py — pandas all-NaN fallback. _process_array_for_pandas fell off the end returning None when interpretation.awkward_form(...) raised CannotBeAwkward, so library="pd" silently built pandas.Series(None, index=...) (all-NaN) instead of the object array. Now returns the array.
  2. identify.py — parse_typename for long long* / unsigned long long*. These branches were dead (shadowed by the scalar long/unsigned long branches, which consume the token) and constructed AsArray({header}, dtype) with the wrong arity. Reordered so the pointer forms are checked first and pass the missing speedbump argument.
  3. identify.py — broken assert message. The assert's f-string message was a separate statement (dead code, truncated message) and validation shouldn't be an assert (stripped under -O). Replaced with a raise ValueError(...).
  4. objects.py — AsStridedObjects:
    • __init__ deleted from a list while iterating range() of its original length (IndexError with mid-list (None, None) header markers from multiple base classes) and could drop the sole member. Rewritten as a filter that drops interior header markers; all_headers_prepended semantics preserved.
    • basket_array mutated shared self._to_dtype based on a basket's byte offsets — a race / state leak across basket-worker threads. The header dtype is now computed locally, and final_array derives the output dtype from the actual basket arrays.
    • Hoisted the per-basket awkward_form(...) recomputation out of the loop, reused has_any_awkward_types, removed a dead container = {}.
  5. strings.py — final_array. output could be referenced unbound for mixed StringArray/awkward baskets; now bound on all paths.
  6. containers.py:
    • STLBitSet.__str__ referenced a nonexistent self._values and __len__ returned an ndarray (both TypeError); fixed to use self._numbytes and return an int.
    • AsBitSet now keeps its bit count and renders std::bitset<N> instead of std::bitset<bool>.
    • Simplified always-true KeysView is not None / ValuesView is not None (Py2 leftovers).
  7. known_forth/init.py — fallback typename = classname_decode(model.__name__) returned a (classname, version) tuple that could never match the string keys in KNOWN_FORTH_DICT; now takes [0].
  8. custom.py — AsBinary (library="np"). The uniformity check numpy.unique(counts[1:] - counts[:-1]).size == 1 failed on single-entry baskets (empty diff → AssertionError) and passed for any arithmetic progression. Changed to numpy.unique(counts).size <= 1 on the per-entry sizes.
  9. python.py — expression compilation. A branch named like a function (e.g. sqrt) used in call position was rewritten to get('sqrt'), breaking sqrt(x). Names in func position that exist in the functions table now resolve to the function and emit NameConflictWarning when shadowed by a branch.
  10. library.py — perf. _strided_to_awkward now uses data.reshape(-1) (view) instead of data.flatten() (copy) at every recursion level.

Skipped

None — all ten findings held up under verification.

Tests

Regression tests added in tests/test_<N>_*.py (named after the PR number) and the targeted/interpretation-heavy suites pass locally.

henryiii added 2 commits June 10, 2026 15:05
…, strided objects, bitset)

Fixes a batch of verified bugs in the interpretation layer:

- library.py: `_process_array_for_pandas` fell off the end returning
  None when `awkward_form` raised CannotBeAwkward, so `library="pd"`
  silently produced an all-NaN Series. Now returns the object array.
- library.py: `_strided_to_awkward` uses `reshape(-1)` (view) instead of
  `flatten()` (copy) at each recursion level.
- identify.py: `parse_typename("long long*")` /
  `"unsigned long long*"` were dead (shadowed by the scalar branches)
  and constructed `AsArray` with the wrong number of arguments. Reordered
  and pass the speedbump argument.
- identify.py: replaced a truncated/dead assert (its f-string message was
  a separate statement) with a proper ValueError.
- objects.py: `AsStridedObjects.__init__` no longer deletes from a list
  while iterating its original range (IndexError) and no longer drops the
  sole member; rewritten as a filter that drops interior (None, None)
  header markers.
- objects.py: `AsStridedObjects` no longer mutates shared `self._to_dtype`
  in `basket_array`; the output dtype is derived locally in `final_array`,
  making the interpretation safe to share across basket-worker threads.
- objects.py: hoisted the per-basket awkward-form recomputation out of the
  loop, reused `has_any_awkward_types`, and removed a dead assignment.
- strings.py: `final_array` now binds `output` on all paths for mixed
  StringArray/awkward baskets (was UnboundLocalError).
- containers.py: `STLBitSet.__str__`/`__len__` now reference the real
  attribute and return an int; `AsBitSet` keeps its bit count and renders
  `std::bitset<N>`; simplified always-true `KeysView`/`ValuesView` checks.
- known_forth/__init__.py: fallback typename now takes `classname_decode(...)[0]`
  (a string) so the known-forth fast path can match.
- custom.py: `AsBinary` numpy uniformity check now tests the per-entry
  sizes themselves (`numpy.unique(counts).size <= 1`), fixing both the
  single-entry AssertionError and the arithmetic-progression false pass.
- python.py: a branch named like a function (e.g. `sqrt`) used in call
  position now resolves to the function and emits NameConflictWarning,
  instead of breaking `sqrt(x)`.

Part of #1646

Assisted-by: ClaudeCode:claude-opus-4-8
Covers: parse_typename("long long*"); pandas object-branch returns data
not NaN; AsBinary single-entry basket and non-uniform rejection; STLBitSet
repr/str/len; AsBitSet typename bit count; expression with a branch named
like a function; AsStridedObjects shared-interpretation has no to_dtype
state leak.

Assisted-by: ClaudeCode:claude-opus-4-8
@codecov

codecov Bot commented Jun 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 87.61905% with 13 lines in your changes missing coverage. Please review.
✅ Project coverage is 73.97%. Comparing base (1c06db0) to head (0776537).
⚠️ Report is 13 commits behind head on main.
✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/uproot/interpretation/identify.py 50.00% 3 Missing and 3 partials ⚠️
src/uproot/interpretation/objects.py 96.66% 1 Missing and 1 partial ⚠️
src/uproot/interpretation/strings.py 0.00% 2 Missing ⚠️
src/uproot/language/python.py 86.66% 1 Missing and 1 partial ⚠️
src/uproot/containers.py 91.66% 0 Missing and 1 partial ⚠️
Additional details and impacted files
Files with missing lines Coverage Δ
src/uproot/interpretation/custom.py 85.52% <100.00%> (+7.89%) ⬆️
src/uproot/interpretation/known_forth/__init__.py 100.00% <100.00%> (ø)
src/uproot/interpretation/library.py 63.96% <100.00%> (+0.42%) ⬆️
src/uproot/containers.py 67.04% <91.66%> (+2.71%) ⬆️
src/uproot/interpretation/objects.py 85.56% <96.66%> (+1.60%) ⬆️
src/uproot/interpretation/strings.py 69.76% <0.00%> (-0.66%) ⬇️
src/uproot/language/python.py 72.32% <86.66%> (-0.89%) ⬇️
src/uproot/interpretation/identify.py 68.82% <50.00%> (+0.70%) ⬆️

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.

1 participant