Skip to content

Fix NaN handling and single-class fit in MovesManagementClassifier and PlannedGivingIntentScorer - #253

Merged
shivamlalakiya merged 4 commits into
mainfrom
f7a-moves-planned
Sep 26, 2026
Merged

shivamlalakiya merged 4 commits into
mainfrom
f7a-moves-planned

Conversation

@shivamlalakiya

Copy link
Copy Markdown
Contributor

What

MovesManagementClassifier was rejecting NaN input even though its
HistGradientBoostingClassifier backend handles missing values natively.
Fitting it on a single-class y also silently produced a classifier whose
predict_proba returned only 1 column instead of failing loudly. This PR:

  • adds allow-nan handling to fit/predict/predict_proba/action_priority and
    the allow_nan estimator tag
  • raises a clear ValueError at fit time when y has fewer than 2 classes
  • removes a dead, already-redundant feature_names_in_ assignment
    (validate_data already sets it)
  • documents that action_priority's confidence is an uncalibrated max
    probability rather than a calibrated likelihood, and that class_weight
    defaults to "balanced"

PlannedGivingIntentScorer calibrates with a fixed cv=2, so any class with
fewer than 2 examples made scikit-learn raise its raw "Requesting 2-fold
cross-validation..." error, which doesn't tell the caller what to do about
it. This PR:

  • raises a clear ValueError before calibration runs when y has fewer than 2
    classes or any class has fewer than 2 examples
  • removes an unreachable branch in predict_intent_score (predict_proba is
    now guaranteed to return 2 columns, since fit rejects fewer than 2 classes)
  • documents that NaN features are rejected, since the GradientBoostingClassifier
    backend doesn't support them

I also looked at whether PlannedGivingIntentScorer.fit should take an
optional sample_weight, since GradientBoostingClassifier supports it. I
tried adding it, but it breaks check_estimator's
check_sample_weight_equivalence_on_dense_data check: CalibratedClassifierCV's
internal cv=2 folding isn't equivalent between a weighted sample and a
repeated one, so passing sample_weight through doesn't behave the way that
check requires. Rather than half-support it, I left fit's signature as
(X, y) only.

How tested

Added regression tests for each behavior fix, confirmed they fail against
the pre-fix code and pass against the fix:

  • test_fit_accepts_nan_features and test_fit_raises_clear_error_on_single_class
    in tests/test_moves.py
  • test_fit_raises_clear_error_when_a_class_has_too_few_examples and
    test_fit_raises_clear_error_on_single_class in tests/test_planned_giving.py

Also removed test_predict_intent_score_single_class_returns_zeros, which
only exercised the now-deleted dead branch by mocking predict_proba's
return shape.

Ran locally with the project's editable dev install:

  • make ci: 2197 passed, 30 skipped, coverage 98.40% (floor 92%)
  • make riskcov: risk-tier coverage 98% (floor 93%)
  • pytest tests/test_sklearn_compliance.py -k "Moves or Planned": all
    passing, including after confirming the poor_score classifier tag isn't
    needed for MovesManagementClassifier (removed it once tests passed without
    it)

Changelog

Added an entry under ## [Unreleased] / ### Fixed in CHANGELOG.md.

…d PlannedGivingIntentScorer

MovesManagementClassifier rejected NaN even though its HistGradientBoostingClassifier
backend handles missing values natively, and fitting on a single-class y silently
produced a classifier whose predict_proba returned only 1 column. It now allows NaN
through fit/predict/predict_proba/action_priority and raises a clear ValueError when
fewer than 2 classes are present. Also dropped a dead, already-redundant
feature_names_in_ assignment and documented that action_priority's confidence is an
uncalibrated max probability, not a calibrated likelihood.

PlannedGivingIntentScorer's fixed cv=2 calibration surfaced scikit-learn's raw
"Requesting 2-fold cross-validation..." error whenever a class had fewer than 2
examples; it now raises a clear ValueError before calibration runs. Also removed an
unreachable branch in predict_intent_score and documented that NaN features are
rejected.
@shivamlalakiya
shivamlalakiya merged commit bb6b169 into main Sep 26, 2026
15 checks passed
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