Skip to content

Commit 4e8bfe0

Browse files
authored
Merge pull request #532 from propcgamer20-png/fix/parse-reference-per-column-scale
fix: decide reference percent-vs-fraction scale per column, not table-wide
2 parents dd37994 + 142eaca commit 4e8bfe0

5 files changed

Lines changed: 84 additions & 10 deletions

File tree

assets/profiler-engine.js

Lines changed: 14 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -856,11 +856,21 @@
856856
if (isNaN(share)) return;
857857
raw.push([String(row[colC]).trim(), String(row[grpC]).trim(), share]);
858858
});
859-
var scale = raw.some(function (r) { return r[2] > 1.5; }) ? 100 : 1;
860-
var reference = {};
859+
// Percent-vs-fraction scale is decided per column (grouped by the column
860+
// identifier), not once across the whole table: a reference file that
861+
// mixes conventions between columns would otherwise get the wrong scale
862+
// applied to whichever column didn't trigger the heuristic. Mirrors
863+
// faircode.profiler.parse_reference.
864+
var byCol = {};
861865
raw.forEach(function (r) {
862-
if (!reference[r[0]]) reference[r[0]] = {};
863-
reference[r[0]][r[1]] = r[2] / scale;
866+
(byCol[r[0]] = byCol[r[0]] || []).push([r[1], r[2]]);
867+
});
868+
var reference = {};
869+
Object.keys(byCol).forEach(function (col) {
870+
var pairs = byCol[col];
871+
var scale = pairs.some(function (p) { return p[1] > 1.5; }) ? 100 : 1;
872+
reference[col] = {};
873+
pairs.forEach(function (p) { reference[col][p[0]] = p[1] / scale; });
864874
});
865875
return reference;
866876
}

faircode/SPEC.md

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -323,7 +323,9 @@ under-sampling relative to who a model will actually serve. Supplied via `--refe
323323

324324
**Format** - a long-format table with three columns (headers case-insensitive; `column`/`dimension`,
325325
`group`/`value`/`label`, `share`/`expected`/`percent`). Shares may be fractions (`0.51`) or
326-
percentages (`51`) - if any value exceeds `1.5` the whole table is read as percentages. Parsed into
326+
percentages (`51`) - the scale is decided per column (rows grouped by the `column` identifier): if
327+
any of a column's values exceeds `1.5` that column is read as percentages, so a reference file that
328+
mixes conventions between columns still parses each column correctly. Parsed into
327329
`{column: {group: expected_share}}`.
328330

329331
```

faircode/profiler.py

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -385,8 +385,11 @@ def parse_reference(df: pd.DataFrame) -> dict:
385385
"""Parse a long-format reference baseline into {column: {group: share}}.
386386
387387
Expected headers (case-insensitive): a column identifier, a group/value, and
388-
a share. Shares may be fractions (0.51) or percentages (51) - if any value
389-
exceeds 1.5 the whole table is read as percentages. See SPEC section 8.
388+
a share. Shares may be fractions (0.51) or percentages (51); the choice is
389+
made per column (grouped by the column identifier) - if any of a column's
390+
values exceeds 1.5 that column is read as percentages. Deciding it once
391+
across the whole table corrupted a correctly-scaled column in a reference
392+
file that mixes conventions between columns. See SPEC section 8.
390393
"""
391394
lower = {str(c).strip().lower(): c for c in df.columns}
392395

@@ -414,10 +417,15 @@ def pick(aliases):
414417
continue
415418
raw.append((str(row[col_c]).strip(), str(row[grp_c]).strip(), share))
416419

417-
scale = 100.0 if any(s > 1.5 for _, _, s in raw) else 1.0
418-
reference: dict = {}
420+
by_col: dict = {}
419421
for col, grp, share in raw:
420-
reference.setdefault(col, {})[grp] = share / scale
422+
by_col.setdefault(col, []).append((grp, share))
423+
424+
reference: dict = {}
425+
for col, pairs in by_col.items():
426+
scale = 100.0 if any(s > 1.5 for _, s in pairs) else 1.0
427+
for grp, share in pairs:
428+
reference.setdefault(col, {})[grp] = share / scale
421429
return reference
422430

423431

tests/test_js_parity.py

Lines changed: 37 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -281,6 +281,43 @@ def test_python_js_reference_parity_on_unmatched_column(tmp_path):
281281
assert "reference file's column(s) don't match any profiled dimension: totally_wrong_col" in completed.stderr
282282

283283

284+
def test_python_js_parse_reference_mixed_scale_parity(tmp_path):
285+
"""parse_reference / parseReference decide percent-vs-fraction per column,
286+
not once across the whole table, so a reference file mixing conventions
287+
between columns parses identically on both engines (#513)."""
288+
from faircode.profiler import parse_reference
289+
290+
ref_df = pd.DataFrame({
291+
"column": ["sex", "sex", "race", "race", "race"],
292+
"group": ["Female", "Male", "White", "Black", "Other"],
293+
"share": [0.6, 0.4, 70, 20, 10],
294+
})
295+
py_result = parse_reference(ref_df)
296+
assert py_result == {
297+
"sex": {"Female": 0.6, "Male": 0.4},
298+
"race": {"White": 0.7, "Black": 0.2, "Other": 0.1},
299+
}
300+
301+
table_json = tmp_path / "table.json"
302+
table_json.write_text(json.dumps({
303+
"columns": list(ref_df.columns),
304+
"rows": ref_df.to_dict(orient="records"),
305+
}), encoding="utf-8")
306+
307+
script = (
308+
"const fs=require('fs');"
309+
"require(process.argv[1]);"
310+
"const t=JSON.parse(fs.readFileSync(process.argv[2],'utf-8'));"
311+
"process.stdout.write(JSON.stringify(globalThis.FairCodeProfiler.parseReference(t)));"
312+
)
313+
completed = subprocess.run(
314+
["node", "-e", script,
315+
str(REPO_ROOT / "assets" / "profiler-engine.js"), str(table_json)],
316+
capture_output=True, text=True, encoding="utf-8", check=True,
317+
)
318+
assert json.loads(completed.stdout) == py_result
319+
320+
284321
def test_python_js_json_parity_inconsistent_keys():
285322
"""Records-orient JSON where later records add columns the first one
286323
doesn't have (#144). The JS parseJSON() used to derive columns from only

tests/test_profiler.py

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -340,6 +340,23 @@ def test_parse_reference_fraction_and_percent():
340340
assert pct == {"sex": {"m": 0.4, "f": 0.6}} # percentages normalized to fractions
341341

342342

343+
def test_parse_reference_mixed_scale_is_decided_per_column():
344+
# A reference file assembled from multiple sources: `sex` given as
345+
# fractions, `race` given as percentages, in the same file. The
346+
# percent-vs-fraction decision used to be made once across the whole
347+
# table, so `race`'s 70 pushed a global scale=100 onto `sex`'s already
348+
# correct 0.6/0.4, corrupting them to 0.006/0.004 (#513).
349+
ref = parse_reference(pd.DataFrame({
350+
"column": ["sex", "sex", "race", "race", "race"],
351+
"group": ["Female", "Male", "White", "Black", "Other"],
352+
"share": [0.6, 0.4, 70, 20, 10],
353+
}))
354+
assert ref == {
355+
"sex": {"Female": 0.6, "Male": 0.4},
356+
"race": {"White": 0.7, "Black": 0.2, "Other": 0.1},
357+
}
358+
359+
343360
def test_parse_reference_percent_string_values():
344361
# "49%" used to raise inside float() and get silently dropped by the
345362
# bare except - the whole --reference file went to {} with no error.

0 commit comments

Comments
 (0)