Add configurable minimum subgroup size warnings - #124
Conversation
|
@ahmdkaml is attempting to deploy a commit to the yashkewlani2020-gmailcom's projects Team on Vercel. A member of the Team first needs to authorize it. |
There was a problem hiding this comment.
🟡 Not ready to approve
It introduces a few fixable maintainability/lint issues in the modified code (formatting/style and duplicate default handling) that should be cleaned up before merging.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
This review doesn't count toward merge requirements. Sign up for the private preview to control whether Copilot approvals count.
Pull request overview
Adds an opt-in/tunable “minimum subgroup size” concept to the dataset profiler output so users are warned when per-group rates are based on small samples, aligning with the request in #101 to reduce over-interpretation of noisy subgroup gaps.
Changes:
- Introduces
min_group_size(default 100) infaircode.profiler.profile()and marks groups withsmall_group. - Renders “small group” warnings in terminal and HTML reports.
- Adds a unit test verifying the default behavior and the tunable override.
File summaries
| File | Description |
|---|---|
tests/test_profiler.py |
Adds coverage for default vs overridden min_group_size behavior. |
faircode/report.py |
Displays small_group warnings in terminal/HTML output. |
faircode/profiler.py |
Implements min_group_size, sets small_group, and emits warning flags. |
faircode/cli.py |
Adds --min-group-size CLI option and passes it into profile() opts. |
Review details
Suppressed comments (1)
faircode/profiler.py:303
- The small-group flag message prints the raw row count without thousands separators, while other parts of the report format counts with commas (e.g.,
(n=...)). Using:,keeps the flag output consistent and more readable for large datasets.
if g.get("small_group"):
flags.append(
f"{d['name']}: '{g['label']}' has only {g['count']} rows; "
f"fairness metrics may be unreliable"
)
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Lite
We're testing this review assessment. Please use 👍 or 👎 to tell us if it's correct.
| share = count / n_nonnull if n_nonnull else 0.0 | ||
| lo, hi = _wilson(count, n_nonnull) | ||
| groups.append({"label": str(label), "count": int(count), "share": share, | ||
| "ci_low": _r(lo, 4), "ci_high": _r(hi, 4)}) | ||
| "ci_low": _r(lo, 4), "ci_high": _r(hi, 4),"small_group": count < min_group_size}) |
| p.add_argument("--min-group-size", type=int, default=100, metavar="N", | ||
| help="warn when a subgroup has fewer than N rows (default: 100)") |
| flags = result["flags"] | ||
| assert not any("fairness metrics may be unreliable" in f for f in flags) | ||
| def test_group_shares_carry_wilson_ci(): |
|
Hey, Will be merging this! if u like the repo pls star it since it helps others find the repo as well! |
|
Thanks for the quick review! I've really enjoyed working through the codebase—I even starred the repo. I have a much better understanding of the project's structure now. If there's a particular issue you'd like prioritized, let me know and I'll work on that next. Otherwise, I'll keep picking issues as I go. |
PR #124 (@ahmdkaml) added the small_group / --min-group-size warning to faircode/profiler.py, cli.py, report.py and tests, but only to the Python engine - breaking the SPEC parity rule that the browser port must produce identical results. Mirror small_group + min_group_size into assets/profiler-engine.js, document it in faircode/SPEC.md (sections 3, 6, 7), and wire the warning into the web UI (profiler-ui.js live view + downloaded HTML report, profiler.css). Fix a couple of PEP8 nits in the Python change. Docs: CHANGELOG [2.0.6], credit @ahmdkaml in the README contributor table, add --min-group-size to the profiler CLI examples, and tighten the CONTRIBUTING parity rule to name result fields (what this PR missed). 113 tests pass; em-dash lint clean; JS engine parses and loads.
Summary
Adds a configurable
--min-group-sizethreshold to warn when subgroup fairness metrics are computed from small sample sizes.Type
Linked issue
Closes #101