Decision tree panic

[slievens]

Fixes #469

Checklist

• My branch is up-to-date with main branch.
• Everything works and tested on latest stable Rust.
• Coverage and Linting have been applied

Current behaviour

Many "predict" and related methods panic when fit hasn't been called.

New expected behaviour

"predict" no longer panics in said case, but returns a Result.

Change logs

Changed

learner	methods
DecisionTreeClassifier	predict, predict_proba
DecisionTreeRegressor	predict
RandomForestClassifier	predict, predict_oob
RandomForestRegressor	predict, predict_oob
ExtraTreesRegressor	predict, predict_oob
KNNClassifier	predict, predict_proba
KNNRegressor	predict
SVC	predict, decision_function
MultiClassSVC	predict
SVR	predict
LinearRegression	predict, predict_matrix
RidgeRegression	predict
Lasso	predict
ElasticNet	predict
LogisticRegression	predict
GaussianNB	predict
BernoulliNB	predict
CategoricalNB	predict
MultinomialNB	predict
XGRegressor	predict

Note

The not-fitted error message of MultiClassSVC::predict changed from "MultiClassSVC is not fitted" to the common "'fit' should be called before calling 'predict'". Code that matches on the old string needs to be updated. See the release notes.

[codecov]

Codecov Report

❌ Patch coverage is 56.55738% with 106 lines in your changes missing coverage. Please review.
✅ Project coverage is 63.65%. Comparing base (9eaae9e) to head (5526859).
⚠️ Report is 191 commits behind head on main.

Files with missing lines	Patch %	Lines
src/svm/svc.rs	48.83%	22 Missing ⚠️
src/ensemble/random_forest_classifier.rs	40.00%	18 Missing ⚠️
src/linear/logistic_regression.rs	68.18%	7 Missing ⚠️
src/neighbors/knn_classifier.rs	68.18%	7 Missing ⚠️
src/ensemble/extra_trees_regressor.rs	25.00%	6 Missing ⚠️
src/linear/lasso.rs	45.45%	6 Missing ⚠️
src/xgboost/xgb_regressor.rs	62.50%	6 Missing ⚠️
src/ensemble/random_forest_regressor.rs	37.50%	5 Missing ⚠️
src/svm/svr.rs	63.63%	4 Missing ⚠️
src/linear/elastic_net.rs	72.72%	3 Missing ⚠️
... and 9 more

Additional details and impacted files

@@             Coverage Diff             @@
##             main     #470       +/-   ##
===========================================
+ Coverage   43.97%   63.65%   +19.67%     
===========================================
  Files          85       96       +11     
  Lines        7281     8513     +1232     
===========================================
+ Hits         3202     5419     +2217     
+ Misses       4079     3094      -985     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[Mec-iS]

@slievens Great contribution — the fix is well-scoped and the consistent error message makes it easy to match on downstream. A few issues to address and suggestions below.

---

Issues to Address

1. Inconsistent guard strategies

Two patterns are used interchangeably with no apparent reason:

• match on Option — extra_trees_regressor, random_forest_regressor, decision_tree_regressor, all Naive Bayes, svc.rs
• Early-return is_none() check — knn_classifier, knn_regressor, svr.rs, all src/linear/*.rs, random_forest_classifier.rs (predict_oob)

The match style is more idiomatic and avoids leaving a latent .unwrap() immediately after the guard. For example in lasso.rs, after the early-return guard, self.coefficients is still unwrapped via self.coefficients(). Recommend standardizing on match throughout.

2. Missing test: predict_oob for RandomForestClassifier

The predict_oob method in random_forest_classifier.rs received a guard but no corresponding test, unlike RandomForestRegressor and ExtraTreesRegressor which both have predict_oob_without_fit_should_not_panic tests.

3. Missing test: predict_proba for DecisionTreeClassifier

predict_proba in decision_tree_classifier.rs received a guard (at line 916), but there is no test for it — only predict is covered.

4. Wrong variable name in DecisionTreeClassifier test

let knn: DecisionTreeClassifier<...> = DecisionTreeClassifier::new();

knn should be tree or dtc — looks like copy-paste from knn_classifier.rs.

5. Missing blank line before test in linear_regression.rs

    }
    #[test]  // ← missing blank line
    fn predict_matrix_without_fit_should_not_panic() {

All other test cases in the file have a blank line before #[test].

---

Suggestions (Non-blocking)

• decision_function in SVR: svr.rs guards predict via self.instances.is_none(), but decision_function (called internally by predict) may still panic if invoked directly. Worth checking whether it also needs a guard.
• Reusable helper: With 17+ identical guard blocks now scattered across the codebase, a small require_fitted() helper method (or a Fitted trait) could centralize this logic and make future learners easier to implement correctly. Could be a good follow-up issue.
• Dedicated error variant: Failed::predict(msg) maps to PredictFailed. A NotFitted variant in FailedError would let callers distinguish "model not fitted" from "prediction failed on valid input" — larger API change, but worth tracking as a future improvement.

[slievens]

Issues 1 to 5 addressed in commit 6e81d2e.

[Mec-iS]

Reviewed head 6e81d2e. The unfitted guards and added regression tests are a useful improvement; the previously requested predict_oob and predict_proba tests are present. One remaining public API gap: SVC::decision_function in src/svm/svc.rs is callable directly and still invokes predict_for_row, which unwraps self.b (and then self.instances). With a nonempty input matrix, SVC::new().decision_function(&x) still panics rather than returning Err(Failed::predict(...)); the new guard in SVC::predict cannot protect direct callers. Could you add a fitted-state check to decision_function and an unfitted regression test? The earlier suggestion about SVR::decision_function appears to refer to this SVC method.

Minor cleanup: Lasso::predict checks self.coefficients.is_none() immediately before matching (&self.coefficients, &self.intercept), so the initial guard is redundant.

I reviewed the diff and CI results, but did not run tests locally.

[slievens]

Both remaining issues have been addressed in 5526859