DOC improve doc for `_check_n_features` and `_check_feature_names` and fix their usages #31585

VirenPassi · 2025-06-18T13:16:42Z

This PR improves the documentation for _check_n_features and _check_feature_names by:

Adding clear docstrings to guide users toward using validate_data(skip_check_array=True) as the preferred public interface.

Including a clarifying comment in FunctionTransformer._check_input explaining that the use of these internal functions aligns with validate_data(..., skip_check_array=True) and is appropriate in this context.

…mes usage Add docstrings suggesting validate_data(skip_check_array=True) as the preferred option. Add inline comment to explain usage in FunctionTransformer.

github-actions · 2025-06-18T13:17:41Z

✔️ Linting Passed

All linting checks passed. Your pull request is in excellent shape! ☀️

_{Generated for commit: 44be546. Link to the linter CI: here}

adrinjalali · 2025-06-19T09:56:58Z

sklearn/preprocessing/_function_transformer.py

+            #  Usage of _check_n_features and _check_feature_names here aligns with
+            # validate_data(..., skip_check_array=True), which avoids full validation.


can't we do validate_data(..., skip_check_array=True) instead?

Thanks! You're absolutely right — in principle, validate_data(..., skip_check_array=True) could replace these internal calls.

In this PR, I kept the original usage to avoid changing existing logic, since the primary goal was documentation improvement and clarification. If preferred, I'm happy to open a follow-up PR that refactors the usage to use validate_data() directly and confirms everything still behaves correctly.

Let me know how you'd like to proceed!

I think it'd be nice to change the logic here in this PR.

Thanks for the suggestion @adrinjalali! I’ve updated _check_input to use validate_data(..., skip_check_array=True) directly as you recommended, replacing the internal checks.

I also tested it locally using Python 3.10.11 — the file test_function_transformer.py passed without any issues. Let me know if anything else looks off!

adrinjalali · 2025-06-20T10:34:00Z

sklearn/preprocessing/_function_transformer.py

-            _check_n_features(self, X, reset=reset)
-            _check_feature_names(self, X, reset=reset)
+            # Use validate_data to perform shape and feature name checks
+            return validate_data(self, X, reset=reset, skip_check_array=True)


I think this whole method (_check_input) can be removed and replaced with a validate_data(..., skip_check_array=self.validate) kinda call.

Hi! Just wanted to note that a few CI jobs are failing due to the stricter validation added by replacing _check_input() with validate_data(...).

Specifically, test_transform_target_regressor_1d_transformer now raises a ValueError because it passes an empty input X, and validate_data correctly catches that.

Let me know if you'd like me to update the test or handle it a different way — happy to make the changes!

…rmer

jeremiedbb

Thanks for the PR @VirenPassi. Here are some comments

jeremiedbb · 2025-06-30T12:38:53Z

sklearn/preprocessing/_function_transformer.py

+            X,
+            reset=True,
+            accept_sparse=self.accept_sparse,
+            dtype="numeric",


This is the default value in check_array so no need to repeat it here.

Suggested change

dtype="numeric",

jeremiedbb · 2025-06-30T12:41:06Z

sklearn/preprocessing/_function_transformer.py

+            reset=True,
+            accept_sparse=self.accept_sparse,
+            dtype="numeric",
+            ensure_all_finite="allow-nan",


Why did you set it to allow-nan ? The default is None which corresponds to True. Note that if validate=False, check_array is skipped so ensure_all_finite is irrelevant.

jeremiedbb · 2025-06-30T12:43:05Z

sklearn/preprocessing/_function_transformer.py

+            accept_sparse=self.accept_sparse,
+            dtype="numeric",
+            ensure_all_finite="allow-nan",
+            ensure_2d=True,


ensure_2d is used in validate_data even if check_array is skipped. So we should condition it to the value of validate as well. This should fix the CI failures

Suggested change

ensure_2d=True,

ensure_2d=self.validate,

jeremiedbb · 2025-06-30T12:44:04Z

sklearn/utils/validation.py

-    .. versionadded:: 1.0
+       This is typically used when `validate=False` during fitting, to still
+       record the feature names.

-    .. versionchanged:: 1.6
-        Moved from :class:`~sklearn.base.BaseEstimator` to
-        :mod:`sklearn.utils.validation`.


Why did you remove this ? It's still usefull imo

jeremiedbb · 2025-06-30T12:44:27Z

sklearn/utils/validation.py


    Parameters
    ----------
    estimator : estimator instance
-        The estimator to validate the input for.
+        The estimator to check or set features name for.


Suggested change

The estimator to check or set features name for.

The estimator to check or set feature names for.

jeremiedbb · 2025-06-30T12:50:28Z

sklearn/utils/validation.py

+       This is typically used when `validate=False` during fitting, to still
+       record the feature names.


It has nothing to do with validate. It always happens unless in very specific cases. and validate is not a common parameter of estimators, only a few of them have it). So I think this sentence is misleading and I would not include it.

Suggested change

This is typically used when `validate=False` during fitting, to still

record the feature names.

jeremiedbb · 2025-06-30T12:51:54Z

sklearn/utils/validation.py

@@ -2706,30 +2706,27 @@ def _to_object_array(sequence):


 def _check_feature_names(estimator, X, *, reset):
-    """Set or check the `feature_names_in_` attribute of an estimator.
+    """Check or set the `feature_names_in_` attribute for an estimator.


I think of was perfectly fine. And why change the order of check and set ?

Suggested change

"""Check or set the `feature_names_in_` attribute for an estimator.

"""Set or check the `feature_names_in_` attribute of an estimator.

jeremiedbb · 2025-06-30T12:53:48Z

sklearn/utils/validation.py

-        .. note::
-           It is recommended to call `reset=True` in `fit` and in the first
-           call to `partial_fit`. All other methods that validate `X`
-           should set `reset=False`.


This not is important. I'd keep it

jeremiedbb · 2025-06-30T12:54:52Z

sklearn/utils/validation.py

-        .. note::
-           It is recommended to call reset=True in `fit` and in the first
-           call to `partial_fit`. All other methods that validate `X`
-           should set `reset=False`.


DOC Improve documentation for _check_n_features and _check_feature_na…

986f464

…mes usage Add docstrings suggesting validate_data(skip_check_array=True) as the preferred option. Add inline comment to explain usage in FunctionTransformer.

github-actions bot added module:preprocessing module:utils labels Jun 18, 2025

VirenPassi mentioned this pull request Jun 18, 2025

Make _check_n_features and _check_feature_names public #30389

Open

adrinjalali reviewed Jun 19, 2025

View reviewed changes

adrinjalali changed the title ~~Improve documentation for _check_n_features and _check_feature_names (#30389)~~ DOC improve doc for _check_n_features and _check_feature_names and fix their usages Jun 19, 2025

github-actions bot added the Documentation label Jun 19, 2025

Use validate_data in _check_input to replace internal checks

d5ea294

adrinjalali reviewed Jun 20, 2025

View reviewed changes

VirenPassi added 2 commits June 20, 2025 19:36

Remove _check_input and directly use validate_data in FunctionTransfo…

460a3e7

…rmer

Replace _check_input with validate_data and fix related test failure

44be546

jeremiedbb reviewed Jun 30, 2025

View reviewed changes

		# Usage of _check_n_features and _check_feature_names here aligns with
		# validate_data(..., skip_check_array=True), which avoids full validation.

	The estimator to check or set features name for.
	The estimator to check or set feature names for.

		This is typically used when `validate=False` during fitting, to still
		record the feature names.

	"""Check or set the `feature_names_in_` attribute for an estimator.
	"""Set or check the `feature_names_in_` attribute of an estimator.

Uh oh!

DOC improve doc for _check_n_features and _check_feature_names and fix their usages #31585

Are you sure you want to change the base?

DOC improve doc for _check_n_features and _check_feature_names and fix their usages #31585

Uh oh!

Conversation

VirenPassi commented Jun 18, 2025

Uh oh!

github-actions bot commented Jun 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✔️ Linting Passed

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VirenPassi Jun 19, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

VirenPassi Jun 20, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jeremiedbb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

DOC improve doc for `_check_n_features` and `_check_feature_names` and fix their usages #31585

DOC improve doc for `_check_n_features` and `_check_feature_names` and fix their usages #31585

github-actions bot commented Jun 18, 2025 •

edited

Loading

VirenPassi Jun 19, 2025 •

edited

Loading

VirenPassi Jun 20, 2025 •

edited

Loading