Skip to content

fix: wire facet_rows/facet_cols roles to score plots - #20

Merged
eilidhmacnicol merged 8 commits into
mainfrom
enh/wire-faceting
Jun 4, 2026
Merged

fix: wire facet_rows/facet_cols roles to score plots#20
eilidhmacnicol merged 8 commits into
mainfrom
enh/wire-faceting

Conversation

@eilidhmacnicol

Copy link
Copy Markdown
Contributor

Goal 4 — complete a half-wired feature (pre-1.0 bug fix)

facet_rows and facet_cols group roles were accepted and validated by the YAML parser (VALID_ROLES, GroupSpec.facet_col_wrap) but nothing in the pipeline consumed them — a config requesting faceting validated cleanly and then produced an unfaceted plot, with no error. This completes the wiring rather than removing the feature.

Design

A FacetGrid is 2D and each latent variable is a distinct result that must always be shown, so LV occupies one axis and at most one demographic facet takes the other:

  • Default: col = LV — unchanged behaviour.
  • facet_rows: G: col = LV, row = G.
  • facet_cols: G: col = G, row = LV (LV is no longer hardwired to columns — addresses the "let the user put LV on rows" case).
  • Both roles: would need three axes → rejected with a clear ValueError.

facet_col_wrap controls LV-column wrapping in the default layout (inert once a row facet exists, per FacetGrid's constraints).

Changes

  • io.GroupConfig: facet_rows_column(), facet_cols_column(), facet_col_wrap() accessors (mirroring hue_column()/x_axis_group()).
  • pipeline._plot_score_boxstrips: resolve the grid layout from those roles and pass row_col/col_wrap through (previously hardwired col_col="LV").
  • Docs: faceting section in docs/usage.md; CHANGELOG fix entry.

Tests (written first, watched fail)

  • test_io: facet accessors return the role columns / None.
  • test_plotting: real FacetGrid gains the expected row/col dimensions (proves the wired params aren't no-ops) — incl. LV-on-rows.
  • test_pipeline::TestFacetWiring: the pipeline maps each config to the correct col_col/row_col; both-roles raises.

All 172 tests pass; ruff clean; coverage 98% (floor 95%).

🤖 Generated with Claude Code

@eilidhmacnicol
eilidhmacnicol merged commit 70517fa into main Jun 4, 2026
4 checks passed
@eilidhmacnicol
eilidhmacnicol deleted the enh/wire-faceting branch June 4, 2026 14:27
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant