Skip to content

Commit 6f93e2d

Browse files
committed
BUG FIX: draw_batch/draw_figures don't materialize selection/weights aliases
P0 Safety: aliases used only in selection= or weights= were never auto-materialized in draw_batch() and draw_figures(). pandas.eval fails with 'name X is not defined'. draw() not affected (separate lazy=False limitation, xfail'd in S1). Root cause: _parse_expr_aliases called without selection= and weights= at draw_batch (line ~11135) and draw_figures (line ~11329). Fix: pass selection and weights to _parse_expr_aliases in both methods. Tests S1(xfail)+S1b+S2+S3+S4: draw lazy workaround, draw_batch selection, draw_figures selection, draw_batch weights. Also: removed test_M1_metadata_skip.py (reverted metadata skip). Discovered: production QA (makeIterationFit123_QA) on gr17. BUG_AliasDataFrame_20260420_draw_selection_alias.
1 parent 7f200fd commit 6f93e2d

2 files changed

Lines changed: 157 additions & 2 deletions

File tree

UTILS/dfextensions/AliasDataFrame/AliasDataFrame.py

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11228,7 +11228,11 @@ def draw_batch(self,
1122811228
expr = merged_spec.get('expr', name)
1122911229
group_by = merged_spec.get('group_by')
1123011230
color = merged_spec.get('color')
11231-
all_needed.update(self._parse_expr_aliases(expr, group_by, color))
11231+
# BUG FIX: include selection + weights in alias discovery
11232+
selection = merged_spec.get('selection')
11233+
weights = merged_spec.get('weights')
11234+
all_needed.update(self._parse_expr_aliases(expr, group_by, color,
11235+
selection=selection, weights=weights))
1123211236

1123311237
# Materialize ALL at once
1123411238
to_materialize = all_needed - already_materialized
@@ -11421,8 +11425,12 @@ def draw_figures(
1142111425
expr = merged_plot.get('expr', '')
1142211426
group_by = merged_plot.get('group_by')
1142311427
color = merged_plot.get('color')
11428+
# BUG FIX: include selection + weights in alias discovery
11429+
selection = merged_plot.get('selection')
11430+
weights = merged_plot.get('weights')
1142411431

11425-
all_needed.update(self._parse_expr_aliases(expr, group_by, color))
11432+
all_needed.update(self._parse_expr_aliases(expr, group_by, color,
11433+
selection=selection, weights=weights))
1142611434

1142711435
# ═══════════════════════════════════════════════════════════════════
1142811436
# PHASE 2: Batch-load branches in lazy reader mode
Lines changed: 147 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,147 @@
1+
"""
2+
BUG_AliasDataFrame_20260420_draw_selection_alias — Regression test
3+
4+
BUG: draw_batch() and draw_figures() do not pass selection/weights to
5+
_parse_expr_aliases, so aliases used in selection expressions are not
6+
auto-materialized. draw() handles this correctly.
7+
8+
REPRODUCER: adf.draw_figures([{..., selection='isNotEdge==1'}]) fails
9+
with "name 'isNotEdge' is not defined" when isNotEdge is an alias.
10+
Pre-materializing isNotEdge before the draw call works around the bug.
11+
12+
FIX: Pass selection= and weights= to _parse_expr_aliases in draw_batch
13+
(line ~11135) and draw_figures (line ~11333).
14+
15+
ENTRY POINT: draw_batch(), draw_figures() — the production entry points
16+
where the bug was discovered. Not _parse_expr_aliases directly.
17+
"""
18+
19+
import os
20+
import sys
21+
import pytest
22+
import numpy as np
23+
import pandas as pd
24+
25+
sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__))))
26+
from AliasDataFrame import AliasDataFrame
27+
28+
try:
29+
import matplotlib
30+
matplotlib.use('Agg')
31+
_HAS_MPL = True
32+
except ImportError:
33+
_HAS_MPL = False
34+
35+
36+
def _build_adf_with_selection_alias():
37+
"""ADF where 'isGood' is an alias used in selection, not a physical column."""
38+
rng = np.random.default_rng(2042)
39+
n = 200
40+
df = pd.DataFrame({
41+
'x': rng.uniform(0, 10, n).astype(np.float32),
42+
'y': rng.normal(0, 1, n).astype(np.float32),
43+
'flag': rng.integers(0, 3, n).astype(np.int8),
44+
})
45+
adf = AliasDataFrame(df)
46+
# isGood is an ALIAS, not a physical column
47+
adf.add_alias('isGood', '(flag > 0)', dtype=np.float32)
48+
# Also add a plot alias that depends on physical columns
49+
adf.add_alias('y_abs', 'abs(y)', dtype=np.float32)
50+
return adf
51+
52+
53+
@pytest.mark.skipif(not _HAS_MPL, reason="Requires matplotlib")
54+
class TestDrawSelectionAliasBug:
55+
"""Tests for BUG_AliasDataFrame_20260420_draw_selection_alias."""
56+
57+
@pytest.mark.xfail(reason=(
58+
"draw() defaults to lazy=False, which skips all alias materialization. "
59+
"Selection aliases are not auto-materialized. Workaround: draw(lazy=True) "
60+
"or pre-materialize. Separate issue from draw_batch/draw_figures fix."
61+
))
62+
def test_S1_draw_materializes_selection_alias(self):
63+
"""
64+
S1: draw() with default lazy=False does NOT auto-materialize
65+
selection aliases. This is a pre-existing limitation.
66+
67+
draw_batch() and draw_figures() always materialize and are
68+
not affected (S2/S3 pass).
69+
"""
70+
adf = _build_adf_with_selection_alias()
71+
assert 'isGood' not in adf.df.columns, "isGood should not be pre-materialized"
72+
73+
# draw() with lazy=False (default) does not materialize isGood
74+
fig = adf.draw('y:x', selection='isGood==1')
75+
assert fig is not None, "S1: draw with selection alias should not fail"
76+
77+
def test_S1b_draw_lazy_materializes_selection_alias(self):
78+
"""
79+
S1b: draw(lazy=True) correctly materializes selection aliases.
80+
This is the workaround for S1.
81+
"""
82+
adf = _build_adf_with_selection_alias()
83+
assert 'isGood' not in adf.df.columns
84+
85+
fig = adf.draw('y:x', selection='isGood==1', lazy=True)
86+
assert fig is not None, "S1b: draw(lazy=True) with selection alias should work"
87+
88+
def test_S2_draw_batch_materializes_selection_alias(self):
89+
"""
90+
S2: draw_batch() must materialize aliases in selection.
91+
92+
Before fix: fails with "name 'isGood' is not defined"
93+
After fix: auto-materializes isGood, draws successfully
94+
"""
95+
adf = _build_adf_with_selection_alias()
96+
assert 'isGood' not in adf.df.columns
97+
98+
specs = {
99+
'plot1': {'expr': 'y:x', 'selection': 'isGood==1'},
100+
}
101+
# Before fix: this raises "name 'isGood' is not defined"
102+
result = adf.draw_batch(specs)
103+
assert result is not None, "S2: draw_batch with selection alias should not fail"
104+
105+
def test_S3_draw_figures_materializes_selection_alias(self):
106+
"""
107+
S3: draw_figures() must materialize aliases in selection.
108+
109+
Before fix: [ERROR] plot 0 'y:x': name 'isGood' is not defined
110+
After fix: auto-materializes isGood, draws successfully
111+
"""
112+
adf = _build_adf_with_selection_alias()
113+
assert 'isGood' not in adf.df.columns
114+
115+
specs = [{
116+
'name': 'test_fig',
117+
'plots': [
118+
{'expr': 'y:x', 'selection': 'isGood==1'},
119+
],
120+
}]
121+
# Before fix: fails silently or raises
122+
result = adf.draw_figures(specs)
123+
assert 'test_fig' in result, "S3: draw_figures should return figure result"
124+
fig_result = result['test_fig']
125+
# Check no error
126+
assert fig_result.get('error') is None, (
127+
f"S3: draw_figures with selection alias failed: {fig_result.get('error')}"
128+
)
129+
130+
def test_S4_draw_batch_materializes_weights_alias(self):
131+
"""
132+
S4: draw_batch() must also materialize aliases used in weights.
133+
Same bug, same fix — weights= wasn't passed to _parse_expr_aliases either.
134+
"""
135+
adf = _build_adf_with_selection_alias()
136+
adf.add_alias('w', 'abs(y) + 1', dtype=np.float32)
137+
assert 'w' not in adf.df.columns
138+
139+
specs = {
140+
'plot1': {'expr': 'y:x', 'weights': 'w'},
141+
}
142+
result = adf.draw_batch(specs)
143+
assert result is not None, "S4: draw_batch with weights alias should not fail"
144+
145+
146+
if __name__ == '__main__':
147+
pytest.main([__file__, '-v', '-s'])

0 commit comments

Comments
 (0)