Skip to content

test(tree): pin min_samples_split boundary and assert depth after #464 - #466

Merged
Mec-iS merged 1 commit into
mainfrom
test/tree-min-samples-split-followup
Sep 23, 2026
Merged

Mec-iS merged 1 commit into
mainfrom
test/tree-min-samples-split-followup

Conversation

@Mec-iS

@Mec-iS Mec-iS commented Sep 23, 2026

Copy link
Copy Markdown
Collaborator

Fixes #465

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

Follow-up to #464 (off-by-one fix in the tree growth loop). Review feedback left two coverage gaps:

  1. No regression test pins the min_samples_split boundary. A node of exactly 3 samples with min_samples_split: 3 must still split under the fixed n >= min_samples_split rule; with min_samples_split: 4 it must stay a single leaf.
  2. The full_depth test asserts nodes().len() == 7 and accuracy but never tree.depth(), although DecisionTreeClassifier::depth() is public API and the growth loop no longer drives the depth counter directly.

New expected behaviour

  • min_samples_split_boundary tests (classifier and base regressor): min_samples_split: 3 on a 3-sample node yields 3 nodes and depth == 2; min_samples_split: 4 yields 1 node and depth == 0.
  • full_depth tests now also assert depth == 3 for both the classifier (tree.depth(), public API) and the base regressor.
  • Patch version bumped to 0.6.15 with a CHANGELOG.md entry. Library code is unchanged.

Change logs

Fixed

Add min_samples_split_boundary regression tests for DecisionTreeClassifier
and BaseTreeRegressor: a node holding exactly min_samples_split samples
must still split, and a node with fewer samples must stay a leaf. The
full_depth tests now also assert tree depth (3), guarding the public
DecisionTreeClassifier::depth accessor. Bump patch version to 0.6.15.

Fixes #465
@Mec-iS
Mec-iS merged commit 3b3b15f into main Sep 23, 2026
13 checks passed
@Mec-iS
Mec-iS deleted the test/tree-min-samples-split-followup branch September 23, 2026 08:52
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.

Add regression tests for min_samples_split boundary and tree depth after #464

1 participant