Skip to content

Commit 6e64a0b

Browse files
author
miranov25
committed
fix(Phase 6.8a): Filter subframe names from ensure_branches() in draw()
BUG-2025-12-11: draw() passed subframe names (e.g., 'SectorCalib') to ensure_branches(), which correctly raised ValueError because they are not TTree branches. Fix: Before calling ensure_branches(), filter out any names that match registered subframes (eager or lazy). - Apply fix to both draw() and draw_batch() - Remove xfail from test_draw_with_subframe - All 1183 tests pass
1 parent d8f24b1 commit 6e64a0b

3 files changed

Lines changed: 22 additions & 16 deletions

File tree

UTILS/dfextensions/AliasDataFrame/AliasDataFrame.py

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9864,6 +9864,11 @@ def draw(self,
98649864
)
98659865
# Load any branches not already loaded
98669866
branches_to_load = required_branches - self._lazy_reader.loaded_branches
9867+
9868+
# Phase 6.8a fix: Filter out subframe names (they are not TTree branches)
9869+
all_subframes = set(self._subframes.subframes.keys()) | set(getattr(self, '_subframe_readers', {}).keys())
9870+
branches_to_load = branches_to_load - all_subframes
9871+
98679872
if branches_to_load:
98689873
self.ensure_branches(list(branches_to_load))
98699874
# =================================================================
@@ -10016,6 +10021,11 @@ def draw_batch(self,
1001610021

1001710022
# Load all required branches at once
1001810023
branches_to_load = all_required - self._lazy_reader.loaded_branches
10024+
10025+
# Phase 6.8a fix: Filter out subframe names (they are not TTree branches)
10026+
all_subframes = set(self._subframes.subframes.keys()) | set(getattr(self, '_subframe_readers', {}).keys())
10027+
branches_to_load = branches_to_load - all_subframes
10028+
1001910029
if branches_to_load:
1002010030
if verbose:
1002110031
print(f"Loading {len(branches_to_load)} branches: {sorted(branches_to_load)}")

UTILS/dfextensions/AliasDataFrame/tests/test_draw_chain_integration.py

Lines changed: 1 addition & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -598,15 +598,10 @@ def test_draw_chain_lazy(self, chain_files):
598598

599599
plt.close(fig)
600600

601-
@pytest.mark.xfail(
602-
reason="BUG-2025-12-11: draw() in lazy mode treats subframe names as TTree branches. Fix in Phase 6.8a"
603-
)
604601
def test_draw_with_subframe(self, single_file):
605602
"""Draw with subframe join.
606603
607-
KNOWN BUG: draw() calls ensure_branches(['SectorCalib']) but SectorCalib
608-
is a subframe name, not a TTree branch. LazyTreeReader correctly raises
609-
ValueError. Fix: Phase 6.8a should filter subframe names from branch list.
604+
Phase 6.8a fixed: draw() now filters subframe names from ensure_branches().
610605
"""
611606
import matplotlib.pyplot as plt
612607
import uproot

UTILS/dfextensions/AliasDataFrame/tests/tests_README.md

Lines changed: 11 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,9 @@
22

33
## Overview
44

5-
**Total Test Files**: 44 (42 Python + 2 C++)
6-
**Total Tests**: ~1190+
5+
**Test Files**: 40 (38 Python + 2 C++)
6+
**Supporting Files**: 4 (conftest.py, __init__.py, run_tests.sh, tests_README.md)
7+
**Total Tests**: ~1183 (as of Phase 6.8e/c)
78
**Last Updated**: 2025-12-11 (Phase 6.8e/c)
89

910
## Running Tests
@@ -72,8 +73,8 @@ pytest tests --ignore=tests/test_draw_invariance.py --ignore=tests/test_draw_cha
7273

7374
| File | Tests | Phase | Description |
7475
|------|-------|-------|-------------|
75-
| `test_draw_invariance.py` | 17 | 6.8e | Draw with known mathematical invariants |
76-
| `test_draw_chain_integration.py` | 26 | 6.8c | Draw across all Phase 7 combinations |
76+
| `test_draw_invariance.py` | 17 | 6.8e | Exact verification: y_derived = 2*x |
77+
| `test_draw_chain_integration.py` | 26 | 6.8c | All Phase 7 loading modes × draw() |
7778
| `test_draw_lazy_integration.py` | 22 | 6/7.3 | Draw with lazy loading |
7879
| `test_ttree_draw_subframe.py` | 10 | 3A | TTree::Draw with friend trees |
7980

@@ -132,7 +133,7 @@ pytest tests --ignore=tests/test_draw_invariance.py --ignore=tests/test_draw_cha
132133
| Lazy Loading | 4 | 189 | 7 |
133134
| Performance Optimization | 4 | ~100 | 8-9 |
134135
| Infrastructure | 11 | ~195 | Various |
135-
| **Total** | **40** | **~1157** | |
136+
| **Total** | **40** | **~1183** | |
136137

137138
---
138139

@@ -176,13 +177,13 @@ Validates TTree::Draw compatibility from C++ side with:
176177

177178
---
178179

179-
## Known Issues
180+
## Known Bugs
180181

181-
### xfail Tests (Expected Failures)
182+
| Bug ID | Test | File | Status | Description |
183+
|--------|------|------|--------|-------------|
184+
| BUG-2025-12-11 | `test_draw_with_subframe` | `test_draw_chain_integration.py` | ✅ Fixed (6.8a) | draw() passes subframe names to ensure_branches(). Fixed by filtering subframe names. |
182185

183-
| Test | File | Reason |
184-
|------|------|--------|
185-
| `test_draw_with_subframe` | `test_draw_chain_integration.py` | BUG-2025-12-11: draw() passes subframe names to ensure_branches(). Fix in Phase 6.8a |
186+
Tests marked `@pytest.mark.xfail` are known issues with documented fix phases. When the fix is implemented, remove the xfail marker and verify the test passes.
186187

187188
---
188189

0 commit comments

Comments
 (0)