fix: CalibrationCurve non-{0,1} binary crash and Weakspots custom-metric override

[hunner]

Pull Request Description

What and why?

Two latent bugs found while empirically reviewing PR #534 (the ZD-704 multiclass fix, shipped in v2.13.6/v2.13.7), reported in that PR's approving review but merged unaddressed. Neither affects the ZD-704 customer (multiclass model, default metrics) — both are follow-on issues of the same family. Tracked as sc-17261.

1. CalibrationCurve crashes on binary targets encoded outside {0, 1}

#534 added a multiclass-only skip (len(np.unique(dataset.y)) > 2 → SkipTestError), but a two-class target whose labels aren't {0, 1} (e.g. {0, 4}, string labels) sails past that guard into sklearn's calibration_curve with no pos_label, hitting the exact cryptic error the guard was meant to eliminate:

ValueError: y_true takes value in {0, 4} and pos_label is not specified: either make y_true take value in {0, 1} or {-1, 1} or pass pos_label explicitly.

2. WeakspotsDiagnosis silently overrides user-supplied custom metrics

apply_averaging (from #534's shared _diagnosis_metrics.py) rebinds every callable in the metric registry that exposes an average parameter — including user-supplied ones. functools.partial(f1_score, average="weighted") still exposes average via inspect.signature, so it gets silently rebound to the resolved averaging (e.g. macro), changing the reported score without any error:

p = functools.partial(f1_score, average="weighted")
bind_averaging(p, "macro", None)(y_true, y_pred)  # returns macro, not the user's weighted score

What changed

• CalibrationCurve.py — passes pos_label=np.unique(dataset.y).max() to calibration_curve, but only when the installed sklearn's signature exposes pos_label (added in scikit-learn 1.1; the library's pyproject.toml has no lower bound, so an unconditional pass would break {0,1} targets on older installs — a regression). The guard mirrors the existing signature-inspection convention in _diagnosis_metrics.bind_averaging. {0,1} behavior is byte-identical (verified against raw calibration_curve output).
• WeakspotsDiagnosis.py — averaging rebinding now only applies when the caller didn't pass custom metrics (i.e. only the defaults get rebound); custom callables keep their own kwargs.

How to test

PYTHONPATH=<worktree> pytest tests/unit_tests/model_validation/sklearn/test_CalibrationCurve.py tests/unit_tests/model_validation/sklearn/test_WeakspotsDiagnosis.py tests/unit_tests/model_validation/sklearn/test_OverfitDiagnosis.py tests/unit_tests/model_validation/sklearn/test_RobustnessDiagnosis.py -q

New coverage: CalibrationCurve on {0,4} and string-label binary (no raise) plus {0,1} unchanged (compared against raw calibration_curve); WeakspotsDiagnosis with a custom partial(f1_score, average="weighted") metric asserting the weighted value lands in the results (with a check that weighted ≠ macro on the test data, so the assertion is meaningful), plus confirmation that default runs still get macro/pos_label rebinding for multiclass and non-{0,1} binary. 41 tests passed across the touched + adjacent (Overfit/Robustness) modules; the three new tests were confirmed to fail against unfixed source with the exact reported errors before the fix, then pass after.

What needs special review?

• The pos_label=np.unique(dataset.y).max() convention follows resolve_averaging's existing "largest label is positive" rule from Fix multiclass / non-{0,1}-label crashes across the sklearn classification tests (ZD-704) #534 — for string labels this is lexicographic max ("good" > "bad"), worth a sanity check against expectations.
• The sklearn-version guard (inspect.signature check for pos_label) rather than an unconditional pass — deliberate, to avoid breaking {0,1} on pre-1.1 sklearn given the unbounded pyproject.toml constraint; CI's actual resolved floor (scikit-learn 1.6.1 on Python 3.9 per uv.lock) means this costs nothing there but protects the documented floor.
• OverfitDiagnosis/RobustnessDiagnosis take a metric name string rather than callables, so they're unaffected by the custom-metrics-override issue — confirmed while implementing, not otherwise touched.

Dependencies, breaking changes, and deployment notes

None. {0,1} CalibrationCurve and default-metric WeakspotsDiagnosis behavior are unchanged; only the two previously-broken/incorrect paths change.

Separately, implementing this surfaced an unrelated pre-existing bug in WeakspotsDiagnosis's pass/fail check (crashes when a custom metrics= subset is passed without matching thresholds=) — filed and fixed independently as sc-17267 / PR #541, not part of this PR.

Release notes

Fixed CalibrationCurve raising a sklearn pos_label error on binary targets not encoded as {0, 1}, and fixed WeakspotsDiagnosis silently overriding user-supplied custom metric averaging (e.g. a partial(f1_score, average="weighted") was previously rebound to macro without warning).

Checklist

• What and why
• How to test
• What needs special review
• Dependencies, breaking changes, and deployment notes
• Labels applied
• PR linked to Shortcut
• Unit tests added
• Tested locally
• Documentation updated (if required)
• Environment variable additions/changes documented (if required)

Closes sc-17261

🤖 Generated with Claude Code

[github-actions]

PR Summary

This PR enhances both the CalibrationCurve and WeakspotsDiagnosis functionality to better handle non-standard binary encodings and custom metrics. Key changes include:

• Adding new unit tests in CalibrationCurve to verify that binary models with non-standard target encodings (e.g., {0, 4} or string labels) no longer trigger errors from scikit-learn due to missing pos_label specification. The tests confirm that backward compatibility is maintained when labels are {0, 1} by ensuring byte-identical results with sklearn's default calibration curve behavior.

• Enhancing the CalibrationCurve function to inspect the signature of sklearn's calibration_curve function and conditionally pass the pos_label parameter. This is to avoid errors on scikit-learn versions that support the parameter while maintaining functionality on older versions.

• Updating WeakspotsDiagnosis to preserve the averaging options for custom user-supplied metrics. The changes ensure that default metrics are rebound (to resolve issues with multiclass/binary handling) but that custom metrics (e.g., using functools.partial with a fixed averaging) retain their original configuration. New tests verify that the custom metric calculations remain true to the user's intent, while default metrics still follow the rebinding logic.

Test Suggestions

• Test CalibrationCurve with additional non-standard binary labels, including negative integers and mixed types.
• Simulate an environment with an older scikit-learn version (pre-1.1) to ensure that the calibration_curve works correctly without pos_label.
• Add tests to ensure that custom metrics using functools.partial retain their user-defined averaging and are not rebinding unexpectedly.
• Validate that the fallback for default metrics still provides the expected macro-averaged values for multiclass scenarios.

[juanmleng]

Two clean fixes — I traced both to root cause and they hold up.

On the CalibrationCurve pos_label change: I checked that this stays correctly oriented, not just crash-free. The stored probability comes from predict_proba(...)[:, 1], which is P(classes_[1]), and for a two-class model classes_[1] is the sorted-max label — the same class np.unique(dataset.y).max() picks. So the probability column and the new pos_label always name the same class ({0,4} → 4, "bad"/"good" → "good"), and the {0,1} case stays byte-identical to before, which your test_standard_binary_curve_unchanged nails down. Guarding the kwarg by signature inspection for pre-1.1 sklearn is the right call given there's no scikit-learn floor.

On the WeakspotsDiagnosis custom-metric change: capturing using_default_metrics = metrics is None before _prepare_metrics_and_thresholds reassigns metrics is exactly right, and only binding averaging on the default path matches the "custom callables own their kwargs" contract. The test that asserts weighted ≠ macro before checking which one lands in the table is a nice touch — it proves the fix scopes averaging correctly rather than just running.

Two small non-blocking things:

• The non-{0,1} calibration tests currently assert only that the run succeeds, not that the curve is oriented correctly. Orientation is actually guaranteed by the [:, 1]/sorted-max alignment above so the risk is low, but if you wanted to fully lock it you could assert the {0,4} curve equals the {0,1} curve on the same data relabeled 1→4 (same probabilities, so the curves must match).
• Worth flagging as a deliberate behavior change: a bare sklearn metric passed as a custom metric on a multiclass target (e.g. metrics={"f1": f1_score} with no average) now raises instead of being silently macro-averaged like it was in v2.13.7. That's the more correct behavior and it's what the contract implies, but the new failure mode is a bit hidden — a one-line docstring note, or catching it and re-raising with a hint to bind average=, would make it discoverable.

Approving — thanks for the thorough tests on both.