Skip to content

Commit a4d175d

Browse files
committed
fix(transformers): probe in fitted order; compare engines with a tolerance — TASK-3
_probe derived both the declared struct schema and the probe's input matrix from the same column list. The struct must be in CALL order (the UDF's Exact signature encodes field order) but .transform must be fed in feature_names_in_ order, the way both engines feed it at runtime. Probing in call order ran the transform on feature-swapped data, so a validating transformer -- e.g. OrdinalEncoder(handle_unknown="error") -- rejected the probe and fit() raised for a query both engines execute correctly. That made the "call order is free" capability this branch documents only conditionally true. _both_engines also compared engine outputs with exact equality, which held only because every fixture landed on exactly-representable arithmetic; an ordinary non-collinear fixture differs by ~2e-16 between the batch and row-at-a-time paths. Now compared with a tolerance, which still catches the whole-feature swaps these tests exist to detect.
1 parent c298ea7 commit a4d175d

2 files changed

Lines changed: 69 additions & 5 deletions

File tree

sql_transform/_transformer_ref.py

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -108,9 +108,20 @@ def _probe(
108108
) -> tuple[pa.Schema, pa.Schema, np.ndarray]:
109109
"""in_schema from `cols`; out_schema by probing .transform; plus the probe's
110110
own output `y`, so a caller that needs the materialised table can build it
111-
without running .transform a second time."""
111+
without running .transform a second time.
112+
113+
`cols` is the STRUCT order -- it must describe the named_struct the SQL
114+
builds, because the UDF is registered with an Exact struct signature and
115+
Arrow struct field order is part of that type. But `.transform` is fed in
116+
the transformer's OWN feature_names_in_ order, which is how both engines
117+
feed it at runtime; probing in struct order would run the transform on
118+
feature-swapped data and make a validating transformer reject a query that
119+
both engines execute correctly. _bind_names guarantees the two orders hold
120+
the same column set, so the reorder always resolves.
121+
"""
112122
in_schema = pa.schema([(c, table.schema.field(c).type) for c in cols])
113-
x = np.column_stack([table.column(c).to_numpy(zero_copy_only=False) for c in cols])
123+
feat = [str(n) for n in obj.feature_names_in_]
124+
x = np.column_stack([table.column(c).to_numpy(zero_copy_only=False) for c in feat])
114125
y = np.asarray(obj.transform(x))
115126
names = [str(n) for n in obj.get_feature_names_out()]
116127
if y.ndim != 2 or y.shape[1] != len(names):

tests/test_transformer_ref.py

Lines changed: 56 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,7 @@
11
"""Differential parity for fitted transformers referenced as {ref} in a t-string."""
22

3+
import math
4+
35
import numpy as np
46
import pandas as pd
57
import pyarrow as pa
@@ -15,11 +17,41 @@
1517
)
1618

1719

20+
def _assert_close(a, b, path=""):
21+
"""Recursively compare two structures (dict/list/scalar), floats within a
22+
tolerance, everything else exactly.
23+
24+
transform (DataFusion) and infer_batch (Rust) take different numerical
25+
paths -- transform stacks the whole batch into one matrix, infer_batch runs
26+
row-at-a-time -- so their floats can differ in the last bit or two (pure
27+
ULP noise, e.g. a non-collinear PCA fixture measured at ~2e-16). That is
28+
not the divergence class these tests exist to catch: the bugs this branch
29+
fixed were whole-feature swaps and hard planning failures, orders of
30+
magnitude above any float tolerance. Exact equality would flake on the
31+
next fixture that isn't lucky enough to land on exactly-representable
32+
arithmetic.
33+
"""
34+
if isinstance(a, dict) and isinstance(b, dict):
35+
assert a.keys() == b.keys(), f"{path}: key mismatch {a.keys()} != {b.keys()}"
36+
for k in a:
37+
_assert_close(a[k], b[k], f"{path}.{k}")
38+
elif isinstance(a, list) and isinstance(b, list):
39+
assert len(a) == len(b), f"{path}: length mismatch {a!r} != {b!r}"
40+
for i, (x, y) in enumerate(zip(a, b, strict=True)):
41+
_assert_close(x, y, f"{path}[{i}]")
42+
elif isinstance(a, float) and isinstance(b, float):
43+
assert math.isclose(a, b, rel_tol=1e-9, abs_tol=1e-12), (
44+
f"{path}: {a!r} != {b!r}"
45+
)
46+
else:
47+
assert a == b, f"{path}: {a!r} != {b!r}"
48+
49+
1850
def _both_engines(t, test_df):
19-
"""transform (DataFusion) and infer (Rust) as plain dicts; assert equal."""
51+
"""transform (DataFusion) and infer (Rust) as plain dicts; assert close."""
2052
batch = t.transform(pa.Table.from_pandas(test_df)).to_pylist()
2153
infer = [r.model_dump() for r in t.infer_batch(test_df.to_dict("records"))]
22-
assert infer == batch, (infer, batch)
54+
_assert_close(infer, batch)
2355
return batch
2456

2557

@@ -395,7 +427,10 @@ def test_transformer_alongside_partitioned_window_agg():
395427

396428
batch = t.transform(pa.Table.from_pandas(train)).to_pylist()
397429
infer = [r.model_dump() for r in t.infer_batch(train.to_dict("records"))]
398-
assert batch == infer
430+
# `o` is StandardScaler output -- same batch-column_stack-vs-row-at-a-time
431+
# risk class as PCA -- so compare with the tolerance helper, not exact
432+
# equality, even though this particular fixture happens to land exact.
433+
_assert_close(infer, batch)
399434
assert [r["m"] for r in batch] == [15.0, 15.0, 35.0, 35.0]
400435

401436

@@ -459,6 +494,24 @@ def test_named_fit_call_order_is_free():
459494
assert np.allclose(got, sc.transform(train))
460495

461496

497+
def test_call_order_free_for_a_validating_transformer():
498+
# The fit-time probe must feed .transform in feature_names_in_ order, the
499+
# way both engines feed it at runtime. Probing in call order ran the
500+
# transform on feature-swapped data, so a transformer that VALIDATES its
501+
# input (OrdinalEncoder with handle_unknown="error") rejected the probe and
502+
# fit() raised -- for a query both engines execute correctly.
503+
df = pd.DataFrame({"cat": ["a", "b", "a", "b"], "grp": ["x", "y", "x", "y"]})
504+
oe = OrdinalEncoder(handle_unknown="error").fit(df) # names: [cat, grp]
505+
506+
t = SQLTransform(t"SELECT {oe}(grp, cat) AS o FROM __THIS__").fit(
507+
pa.Table.from_pandas(df)
508+
) # CALL order reversed
509+
batch = _both_engines(t, df)
510+
expected = oe.transform(df)
511+
got = np.array([[r["o"]["cat"], r["o"]["grp"]] for r in batch])
512+
assert np.allclose(got, expected)
513+
514+
462515
def test_nested_outer_fitted_in_permuted_order_parity():
463516
# The outer's fitted order is a PERMUTATION of the inner's output names. The
464517
# struct the outer actually receives is the inner's output, so in_schema must

0 commit comments

Comments
 (0)