Skip to content

Commit ae18644

Browse files
authored
fix(generate): fail loudly on a headerless .wgt.csv source, don't misparse it (#194)
## Summary - Reported ask was ".wgt.csv weighted source at the attribute level, not just whole-entity" - turned out inverted: `<key source="x.wgt.csv">` (attribute level) already applies weights correctly (verified statistically: 79% vs 80% expected on a 3000-row sample). The real, confirmed bug is the opposite end: `<generate source="x.wgt.csv">` (entity level) silently misparses the headerless value|weight format as a headered CSV, producing nonsense column names and fewer rows than the file has. - Full fix (wiring `WeightedEntityDataSource` into `<generate>`'s chunked pagination) is bigger and riskier than it first looks: `ChunkSourceReader`'s `loads_all` path (what a weighted source would need) re-permutes its pool via `get_distributed_data`/`get_unique_data` - reusing it would double-apply selection on top of already-weighted draws, and a naive per-page rebuild risks silently duplicating rows across multiprocess workers. That's real surgery on shared pagination code, scoped as follow-up work, not this PR. - This PR only fails loudly instead of silently producing garbage for `.wgt.csv` at `<generate>`-level. - `.wgt.ent.csv` (a normal headered CSV with an extra "weight" column) is explicitly NOT touched - reading it plainly (no resampling, weight column as literal data) is coherent and an existing test (`test_source_script/test_iterate_source_scripted.xml`) already relies on exactly that for an unrelated feature. Confirmed it still passes. ## Test plan - [x] TDD: failing tests for both `.wgt.csv`-must-raise and `.wgt.ent.csv`-must-still-read-plainly, confirmed RED/GREEN. - [x] Existing `test_iterate_source_script` (which reads a `.wgt.ent.csv`-named file for an unrelated reason) still passes. - [x] Full suite: 1428 passed, 15 skipped. - [x] mypy/ruff clean.
1 parent 0a5e4f3 commit ae18644

6 files changed

Lines changed: 65 additions & 0 deletions

File tree

datamimic_ce/tasks/task_util.py

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -344,6 +344,20 @@ def gen_task_load_data_from_source_or_script(
344344
else:
345345
# Evaluate script in source
346346
source_data = context.evaluate_python_expression(stmt.script)
347+
elif source_str.endswith(".wgt.csv"):
348+
# A ".wgt.csv" file is headerless (value|weight, no column names) - the plain CSV
349+
# reader below would treat its first data row as a header, producing nonsense column
350+
# names and one fewer row than the file has. Unlike ".wgt.ent.csv" (a normal headered
351+
# CSV that merely has an extra "weight" column - reading it plainly is coherent, just
352+
# unweighted), there is no coherent plain-CSV reading of this format at all. <key
353+
# source="...wgt.csv"> already applies its weights correctly; <generate>-level
354+
# weighted-entity sourcing is real work, not yet done - fail loudly instead of
355+
# silently generating garbage.
356+
raise ValueError(
357+
f"<generate> '{stmt.full_name}': source '{source_str}' is a headerless weighted "
358+
f"value|weight file - not supported at <generate>-level (only <key source=...> "
359+
f"applies '.wgt.csv' weights today)"
360+
)
347361
# Load data from CSV
348362
elif source_str.endswith(".csv"):
349363
source_data = DataSourceRegistry.load_csv_file(
Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,2 @@
1+
true|80
2+
false|20
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
id,name,weight
2+
1,Cheap,80
3+
2,Expensive,20
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<setup>
2+
<generate name="rows" source="data/products.wgt.ent.csv" separator="," count="5" target="JSON"/>
3+
</setup>
Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
<setup>
2+
<generate name="rows" source="data/active.wgt.csv" count="5" target="JSON"/>
3+
</setup>
Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,40 @@
1+
# DATAMIMIC
2+
# Copyright (c) 2023-2025 Rapiddweller Asia Co., Ltd.
3+
# This software is licensed under the MIT License.
4+
# See LICENSE file for the full text of the license.
5+
# For questions and support, contact: info@rapiddweller.com
6+
7+
"""<generate source="x.wgt.csv"> (the headerless value|weight format) falls through to the plain
8+
CSV reader, which has no coherent way to read it - it would treat the first data row as a header,
9+
producing nonsense column names and one fewer row than the file has. Full weighted-entity support
10+
at <generate>-level needs pagination-core work (see PR #194 discussion); until that lands, fail
11+
loudly instead of silently producing garbage.
12+
13+
".wgt.ent.csv" is different: it's a normal headered CSV that happens to carry an extra "weight"
14+
column, so reading it plainly (ignoring the weight, no resampling) is coherent - just unweighted.
15+
An existing test (test_source_script/test_iterate_source_scripted.xml) already relies on exactly
16+
that, so this must keep working."""
17+
18+
from pathlib import Path
19+
20+
import pytest
21+
22+
from datamimic_ce.data_mimic_test import DataMimicTest
23+
24+
_dir = Path(__file__).resolve().parent
25+
26+
27+
def test_generate_source_wgt_csv_raises():
28+
engine = DataMimicTest(_dir, "generate_wgt_plain.xml", capture_test_result=True)
29+
with pytest.raises(ValueError, match=r"\.wgt\.csv"):
30+
engine.test_with_timer()
31+
32+
33+
def test_generate_source_wgt_ent_csv_reads_as_plain_csv():
34+
"""Not weighted (no resampling, no bias toward the higher-weight row) - just an ordinary
35+
headered CSV read, weight column included as literal data. Must NOT raise."""
36+
engine = DataMimicTest(_dir, "generate_wgt_ent.xml", capture_test_result=True)
37+
engine.test_with_timer()
38+
rows = engine.capture_result()["rows"]
39+
assert {row["name"] for row in rows} <= {"Cheap", "Expensive"}
40+
assert all("weight" in row for row in rows)

0 commit comments

Comments
 (0)