Skip to content

Make both plot helpers backend-agnostic via a public pr_band - #7

Merged
hamed merged 3 commits into
mainfrom
feat/backend-agnostic-plotting
Aug 24, 2026
Merged

hamed merged 3 commits into
mainfrom
feat/backend-agnostic-plotting

Conversation

@hamed

@hamed hamed commented Aug 23, 2026

Copy link
Copy Markdown
Owner

box_plot took pandas and plot_pr took Spark. The two plot helpers demanded opposite backends, neither matched the dispatching core, and a pandas user could not call plot_pr at all.

The Spark code moved rather than went away

The Spark calls inside plot_pr were not incidental. They reduced the curve table — 100 replicas by every distinct recall value — to plot size before anything reached the driver. Deleting them would send a large input to toPandas() whole.

So the reduction became a dispatched metric function. pr_band now sits beside confusion_table and calculate_pr, with an implementation for each backend:

from replicas import pr_band

band = pr_band(kpi, by="name", ci=0.90)  # name, recall, precision, low, high

It is public rather than private because the README says most users feed the metric output into their own plotting code, and those users want the band numbers, not the picture.

plot_pr calls it and draws. box_plot converts at the door, since at() has already reduced the data to one row per group and replica. Both now accept pandas, Polars, or Spark.

PySpark floor 3.3 → 3.5

pr_band uses F.percentile, added in PySpark 3.5. This is not a cosmetic preference — percentile_approx returns a different order statistic:

q=0.05 q=0.95
pandas quantile -1.2899645000000002 1.6916200499999998
Polars quantile(interpolation="linear") -1.2899645000000002 1.6916200499999998
Spark F.percentile -1.2899645 1.6916200499999998
Spark percentile_approx -1.320431 1.689107

On 100 samples that is 2.4% off at the lower edge — a visible shift in a plotted band, not a rounding difference. Polars needed interpolation="linear" for the same reason; it defaults to "nearest", which gives a third answer. With both fixed, the three backends agree to within floating-point error, and test_pr_band_agrees_across_backends pins the interpolated values.

Spark 3.3 and 3.4 are both past end of life. The pandas UDF fallback stays: it is gated on applyInArrow at Spark 4.1, so it still serves 3.5-4.0.

Two CI jobs were already red on main — both fixed here

I should flag this plainly: the notebook job I added in #5 has been failing since it merged. I verified it locally and did not check the run. Two separate causes:

  1. The address normalizer mangled figure dimensions. 0x[0-9a-fA-F]+ matches the 0x600 inside <Figure size 730x600 ...>, rewriting a dimension into the address placeholder. The pattern now requires a word boundary.
  2. Figure dimensions are not stable across matplotlib releases. CI produced 730x600 where this machine produced 718x600, from legend spacing alone. Comparing them reports a dependency bump as a defect. Dimensions are now normalized away; the axes count in the same repr is still compared, so a facet that stops rendering still fails.

minimum-dependencies was also red, from before this series: tests/test_plotting.py imported matplotlib at module scope and that job installs no plotting extra. It now skips.

Notebook

examples/quickstart.ipynb drops two .toPandas() calls that are no longer needed and is re-executed. Three of its seven figures changed — the plot_pr bands moved from approximate to exact percentiles. Every text output is unchanged.

Verification

  • ruff check . / ruff format --check . — clean.
  • pytest -q127 passed (108 before; 19 new).
  • scripts/check_example_notebook.py — clean, matches committed outputs.
  • At the exact minimum floor (pyspark==3.5.0 pandas==1.3.0 polars==1.0.0 pyarrow==4.0.0 numpy==1.21.0, Python 3.9), the pr_band and metrics tests pass. That validates the three riskiest choices: F.percentile on 3.5.0, pandas 1.3 named aggregation on a SeriesGroupBy, and the polars 1.0 how="full", coalesce=True join.

Caveat on that last point: I could not run the Spark sampling tests at the floor locally. This machine has only Java 21 and Spark 3.5 requires Java 8/11/17 — they fail with UnsupportedOperationException: sun.misc.Unsafe ... not available. A control run confirms the limit is environmental rather than a regression: the old 3.3.0 floor fails worse on the same Java, erroring at session creation. The minimum-dependencies job uses Java 11, so CI is the real check on that job.

🤖 Generated with Claude Code

hamed and others added 3 commits August 24, 2026 01:43
box_plot took pandas and plot_pr took Spark. The two plot helpers
demanded opposite backends, neither matched the dispatching core, and a
pandas user could not call plot_pr at all.

The Spark code in plot_pr was not incidental: it reduced the curve table
to plot size before anything reached the driver. So it moved rather than
went away. pr_band is now a public metric function beside confusion_table
and calculate_pr, with an implementation for each backend. It returns the
original curve and a pointwise quantile band across the replicas, and it
is worth exposing on its own -- the README says most users feed the
metric output into their own plotting code, and those users want the
numbers, not the picture.

plot_pr calls pr_band and draws. box_plot converts at the door, since
at() has already reduced the data. Both accept pandas, Polars, or Spark.

The spark extra now requires PySpark 3.5, up from 3.3, for F.percentile.
percentile_approx returns a different order statistic: on 100 replicas
it put the 5th-percentile band edge 2.4% off the pandas and Polars
value. That is a visible shift in a plotted band, not a rounding
difference. Polars needs interpolation="linear" for the same reason --
it defaults to "nearest". With both, the three backends agree on the
band to within floating-point error, which the conformance test pins.

Also fixes two CI jobs that were already red on main:

- minimum-dependencies failed collection because tests/test_plotting.py
  imported matplotlib at module scope, which that job does not install.
  It now skips.
- The notebook job compared figure dimensions, which move between
  matplotlib releases: CI produced 730x600 where this machine produced
  718x600. The address normalizer also mangled them, because the 0x600
  inside 730x600 matches a hex literal. Dimensions are now normalized
  away and the address pattern requires a word boundary. The axes count
  in the same repr is still compared.

examples/quickstart.ipynb drops two now-unnecessary .toPandas() calls
and is re-executed. Three of its seven figures changed: the plot_pr
bands moved from approximate to exact percentiles. Every text output is
unchanged.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Self-review of the new code found a defect the tests missed: every
fixture row used the same non-null group key.

pr_band joined the original curve to the replica band on the grouping
columns plus recall. The three backends do not agree on whether a join
matches null keys -- pandas matches NaN to NaN, Polars defaults to
nulls_equal=False, and Spark's join on a column list uses null-unsafe
equality. So a null in a `by` column gave one correct row on pandas and
two half-filled rows on the other two, splitting a curve from its band.
plot_pr would then draw a curve with no band beside a band with no
curve. This contradicted the module docstring, which says a null
grouping value is an ordinary group.

Rather than patch three join dialects, the join is gone. Each backend
now reaches the result with one grouped aggregation over masked or
filtered values: max of the precision where replica is -1, and the two
quantiles over the replicas. That also removes a shuffle, and it keeps
the three implementations spelling out the same small algorithm.

Two regression tests cover it across all three backends: a null group
key, and a recall the original reaches but no replica does.

The null-key test builds its frame from an explicit schema. An all-null
column with no declared type is dtype Null on Polars, and polars 1.0
panics when it groups on that -- a fixture artifact, not something a
caller with a typed column hits.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
replicas[polars,plot] declares no PyArrow, and polars.DataFrame.to_pandas
goes through Arrow, so both plot helpers raised ModuleNotFoundError in
exactly the environment the README told a Polars user to install. The
claim that the plot extra is sufficient on top of any backend was false.

Verified in a clean [polars,plot] venv: pandas arrives transitively
through seaborn, PyArrow does not arrive at all, and both box_plot and
plot_pr failed.

_to_pandas now falls back to a column-wise copy when PyArrow is missing,
rather than adding PyArrow to the plot extra and charging every pandas
plotting user for a conversion only Polars needs. The data is already
reduced to plot size at that point, so the slower path costs nothing
that matters, and the fast path is unchanged wherever PyArrow exists.

The plot extra now declares pandas outright. It arrived through seaborn
either way; both helpers build a pandas frame, so leaning on a
transitive dependency for it was luck, not design.

Two regression tests pin the pyarrow-free path by making find_spec
report it missing.

Also trims the pr_band docstring, which had grown past the size of the
operation it describes.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@hamed

hamed commented Aug 24, 2026

Copy link
Copy Markdown
Owner Author

The Polars blocker was real and is fixed in 9698371. CI green on that commit. Replies to each point below.

1. Polars plotting dependencies — fixed

You were right, and the problem was slightly worse than described: it hit both helpers, not just box_plot, since plot_pr collects the reduced pr_band result the same way.

I built the exact environment you named to pin down which dependency was missing:

$ uv pip install -e '.[polars,plot]'
matplotlib 3.11.1   numpy 2.5.2   pandas 3.0.5   polars 1.43.2   seaborn 0.13.2
$ python -c "...; box_plot(polars_df)"
box_plot(polars): FAIL -> ModuleNotFoundError: No module named 'pyarrow'
plot_pr(polars):  FAIL -> ModuleNotFoundError: No module named 'pyarrow'

So pandas is present — seaborn pulls it — and PyArrow is the actual gap, because polars.DataFrame.to_pandas goes through Arrow.

Of your two suggested resolutions I took neither exactly, because both charge someone who does not benefit. Adding PyArrow to the plot extra bills every pandas plotting user for a conversion only Polars needs; requiring pandas explicitly in the docs would leave the README's "the plot extra is all you need" claim false.

Instead _to_pandas falls back to a column-wise copy when PyArrow is absent:

collect = getattr(df, "to_pandas", None)
if callable(collect):
    if find_spec("pyarrow") is not None:
        return collect()
    import pandas as pd
    return pd.DataFrame(df.to_dict(as_series=False))

The data is already reduced to plot size by then — that is the whole point of the pr_band split — so the slower path costs nothing that matters, and the Arrow path is unchanged wherever PyArrow exists. Re-verified in the same clean venv: both helpers now work, and pr_band still returns a native Polars frame.

I did also declare pandas in the plot extra. It arrived through seaborn either way, but both helpers build a pandas frame, so depending on that transitively was luck rather than design.

Two regression tests pin the path by making find_spec report PyArrow missing.

3. recall_round — I want to push back on this one

This is the point where I disagree, and I think it is a factual disagreement rather than a taste one.

recall_round cannot move to plotting. Plotting only ever sees the reduced frame. Rounding has to happen before the quantiles are taken, because replica curves rarely land on identical recall values — once you have grouped by exact recall and computed quantiles, every group holds one replica, the band collapses, and no downstream rounding recovers it.

So it is not an implementation detail leaking upward. It is a modelling decision — how finely to discretize the recall axis before comparing replicas — that is only expressible at the point of reduction. The default is None, which does nothing and keeps full curve resolution; a user who never thinks about it gets the honest answer.

I have made the docstring say this explicitly rather than describing it as a convenience.

2 and the API surface — partly taken

Agreed that the documentation had outgrown the operation. I cut the pr_band docstring by about a third.

I kept the validation. pr_band uses the same _groups / _validate_columns / reserved-column checks as confusion_table and calculate_pr — those are shared helpers, not new machinery, and removing them would make pr_band the odd one out rather than the lean one. The genuinely new surface is one function, one return schema, and one backend method each.

4. Spark floor — the requirement is hard

Cross-backend numerical equivalence is the README's headline claim, not an optional goal: "equivalent inputs produce the same source-row multiplicities across backends", with an explicitly documented carve-out list. A confidence band that differs by 2.4% depending on which engine you ran it on belongs on that carve-out list or gets fixed. I would rather fix it.

Verification

  • pytest -q135 passed.
  • ruff check . / ruff format --check . — clean.
  • Notebook checker — clean, matches committed outputs.
  • CI on 9698371 — all nine jobs green, including minimum-dependencies and notebook.
  • Clean [polars,plot] venv with no PyArrow — both helpers work.

@hamed
hamed merged commit b217bc9 into main Aug 24, 2026
9 checks passed
@hamed
hamed deleted the feat/backend-agnostic-plotting branch August 24, 2026 07:42
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