Skip to content

[ENH] Add second FeatureSelection test parameter set - #10232

Merged
fkiraly merged 3 commits into
sktime:mainfrom
snoopuppy582:enh/feature-selection-test-params-3429
Aug 16, 2026
Merged

fkiraly merged 3 commits into
sktime:mainfrom
snoopuppy582:enh/feature-selection-test-params-3429

Conversation

@snoopuppy582

@snoopuppy582 snoopuppy582 commented May 12, 2026 •

Copy link
Copy Markdown
Contributor

Reference Issues/PRs

Part of #3429.

What does this implement/fix? Explain your changes.

This adds a second lightweight test parameter set for FeatureSelection and removes it from the test_get_test_params_coverage exclusion list.

The additional parameter set uses method="feature-importances" with n_columns=2. This exercises the supervised feature-importance branch without adding a soft dependency or expensive fitting.

Does your contribution introduce a new dependency? If yes, which one?

No.

What should a reviewer concentrate their feedback on?

Whether the feature-importance configuration is an appropriate second lightweight test parameter set for FeatureSelection.

Did you add any tests for the change?

  • check_estimator(FeatureSelection) -> total=80, failed=0
  • TestAllObjects().run_tests(FeatureSelection, tests_to_run="test_get_test_params_coverage") -> total=1, failed=0
  • ruff check sktime/transformations/feature_selection.py sktime/tests/_config.py -> All checks passed

@OfficialAbhinavSingh

Copy link
Copy Markdown
Contributor

method="none" doesn't pass the suite. running check_estimator(FeatureSelection) with these params gives 4 failures, all on param set 1:

test_fit_transform_output: FeatureSelection.transform should return an object of
scitype Series when given an input of scitype Series, but found the following
return: None

test_fit_idempotent / test_persistence_via_pickle / test_save_estimators_to_file:
unsupported operand type(s) for -: 'NoneType' and 'NoneType'

"none" returns None from transform by design, so the boilerplate can't take it as a Series.

{"method": "feature-importances", "n_columns": 2} passes all 80 checks and exercises the regressor path instead of the passthrough.

also worth a rebase, the _config.py hunk is against a stale main (DontUpdate is no longer in that list).

@snoopuppy582
snoopuppy582 force-pushed the enh/feature-selection-test-params-3429 branch from fb8b3e2 to 3b1136a Compare August 4, 2026 04:18
@snoopuppy582

Copy link
Copy Markdown
Contributor Author

Thanks for catching this. I reproduced the four failures with method="none" and confirmed that the None output is incompatible with the estimator checks.

I rebased onto the current main, replaced the second parameter set with {"method": "feature-importances", "n_columns": 2}, and kept the config change limited to removing FeatureSelection from the coverage exclusion.

Validation after the update:

  • check_estimator(FeatureSelection): 80/80 passed
  • test_get_test_params_coverage: passed
  • Ruff on both changed files: passed

The updated branch is pushed.

@fkiraly fkiraly added module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing enhancement Adding new functionality labels Aug 16, 2026
@fkiraly
fkiraly merged commit 7fc0a72 into sktime:main Aug 16, 2026
1 check was pending
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement Adding new functionality module:transformations transformations module: time series transformation, feature extraction, pre-/post-processing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants