Skip to content

Add the n_feature_parts parameter to the supervised estimators#544

Open
tramora wants to merge 1 commit intomainfrom
533-add-max-parts-supervised-estimators
Open

Add the n_feature_parts parameter to the supervised estimators#544
tramora wants to merge 1 commit intomainfrom
533-add-max-parts-supervised-estimators

Conversation

@tramora
Copy link
Collaborator

@tramora tramora commented Feb 3, 2026

  • KhiopsClassifier, KhiopsRegressor and KhiopsEncoder

Fixes #533


TODO Before Asking for a Review

  • Rebase your branch to the latest version of main (or main-v10)
  • Make sure all CI workflows are green
  • When adding a public feature/fix: Update the Unreleased section of CHANGELOG.md (no date)
  • Self-Review: Review "Files Changed" tab and fix any problems you find
  • API Docs (only if there are changes in docstrings, rst files or samples):
    • Check the docs build without warning: see the log of the API Docs workflow
    • Check that your changes render well in HTML: download the API Docs artifact and open index.html
    • If there are any problems it is faster to iterate by building locally the API Docs

type_error_message("n_feature_parts", self.n_feature_parts, int)
)
if self.n_feature_parts < 0:
raise ValueError("'n_feature_parts' must be positive")
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

positive -> non-negative

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You are totally right, this reformulation would be more precise. Unfortunatly the message is duplicated many times and for consistency sake it would be required to modify everywhere (in a distinct PR)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A commit for fixing this would suffice. It is a small change for a PR.

"specific_pairs": [],
"all_possible_pairs": False,
"construction_rules": ["TableMode", "TableSelection"],
"max_parts": 4,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make one of these tests with an explicit 0

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, I changed the test for KhiopsClassifier "fit" with a monotable : the n_feature_parts parameter was not provided and thus the default value of 0 is set

CHANGELOG.md Outdated
## Unreleased

### Added
- (`sklearn`) A `n_feature_parts` parameter to the supervised estimators
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eliminate A

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed

- KhiopsClassifier, KhiopsRegressor and KhiopsEncoder
@tramora tramora force-pushed the 533-add-max-parts-supervised-estimators branch from bef0798 to d084a67 Compare February 4, 2026 10:59
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.

Support for max_parts in sklearn supervised estimators

2 participants