ENH Add support for np.nan values in SplineTransformer #28043

StefanieSenger · Jan 2, 2024

Reference Issues/PRs

Closes #26793

What does this implement/fix? Explain your changes.

Adds support for np.nan values in SplineTransformer.

adds param handle_missing : {'error', 'constant'} to init, where error preserves the previous behaviour and constant handles nan values by setting their spline values to all 0s
adds new test (but needs to be extended)

Yet to solve:

I believe in _get_base_knot_positions I have to prepare _weighted_percentile for excluding nan values similarity to how np.nanpercentile excludes nan values for the calculation of the base knots. I tried, but it was quite tricky. Edit: Just found that np.nanpercentile will have a sample_weight option soon: PR 24254 in numpy
Should an error also be raised in case the SplineTransformer was instantiated with (handle_missing="error"), then fitted without missing values and the X then contains missing values in transform?

github-actions · Jan 2, 2024

✔️ Linting Passed

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

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

ogrisel

The PR looks very good but it needs to be merged with main (there are conflicts in the changelog).

Also, I think the get_output_feature_names() method needs to be updated. The tests should be expanded accordingly, maybe to also include a test with .set_output(transform="pandas") (this is how I found out that there was a problem with the output feature names).

ogrisel · Feb 29, 2024

So, should sparse_output=True and handle_missing="indicator" be prevented from being used together (and show an explicit error message) ,

I think we should add support for using those two options together.

ogrisel

Here is a more in depth pass of review. There is indeed a fundamental problem with the current code: the missingness indicators from the training set (when calling .fit or .fit_transform) should not be stored as an estimator attribute and reapplied to the test set (when calling .transform). Instead the missingness pattern from the test set should be extracted.

See more details below:

sklearn/preprocessing/_polynomial.py

ogrisel · Mar 1, 2024

sklearn/preprocessing/_polynomial.py

        if self.include_bias:
-            return XBS
+            return self._concatenate_indicator(XBS)


The missingness indicators computed from the X passed to .transform (which can be a test set) should be passed as argument to _concatenate_indicator instead of reusing the mask extracted from the training set.

sklearn/preprocessing/tests/test_polynomial.py

StefanieSenger

Hey @ogrisel, thanks for reviewing and your help.
I went through your comments and could resolve most of the issues.

I've named the new option handle_missing="constant", but that's just an idea. I found that indicator doesn't fit so well anymore, if we don't add an indicator column to X. Though with constant as well as with zeros I feel that it's not quite clear from the naming, where in the process the nans become something else (before or after calculating the splines). Maybe we can find a name, that conveys that info.

There are quite a few things, I am a bit confused about:
Generally, I don't know if we want SplineTransformer to change or keep behaviour if nan values are present.

If we want it to keep behaviour, instead of having this test data for comparing equality:

    X_nan = np.array([[1, 1], [2, 2], [3, 3], [np.nan, 4], [4, 4]])
    X = np.array([[1, 1], [2, 2], [3, 3], [4, 4]])

it should maybe rather be

    X_nan = np.array([[1, 1], [2, 2], [3, 3], [np.nan, 4], [4, 4]])
    X = np.array([[1, 1], [2, 2], [3, 3], [99, 4], [4, 4]])

and in this case, the current implementation is wrong. Maybe you can shed a light on this so that I know how to go on.

I will check the issue with the feature names next.

sklearn/preprocessing/_polynomial.py

sklearn/preprocessing/tests/test_polynomial.py

StefanieSenger · Mar 15, 2024

I was trying to find about the problem with the feature names, that you have mentioned here, @ogrisel, but I cannot recreate it. Maybe it's been resolved when I worked on the other issues?

This is what I tried (using the code from the existing feature_name_out test):

def test_spline_transformer_feature_names_with_nans():
    """Test that SplineTransformer generates correct feature names if nan values are present."""
    X_nan = np.array([[1, 1], [2, 2], [3, 3], [np.nan, 4], [4, 4]])
    splt = SplineTransformer(
        degree=3,
        n_knots=3,
        handle_missing="constant",
        include_bias=True).fit(X_nan)
    feature_names = splt.get_feature_names_out()
    assert_array_equal(
        feature_names,
        [
            "x0_sp_0",
            "x0_sp_1",
            "x0_sp_2",
            "x0_sp_3",
            "x0_sp_4",
            "x1_sp_0",
            "x1_sp_1",
            "x1_sp_2",
            "x1_sp_3",
            "x1_sp_4",
        ],
    )

    splt = SplineTransformer(
    degree=3,
    n_knots=3,
    handle_missing="constant",
    include_bias=False).fit(X_nan)
    feature_names = splt.get_feature_names_out(["a", "b"])
    assert_array_equal(
        feature_names,
        [
            "a_sp_0",
            "a_sp_1",
            "a_sp_2",
            "a_sp_3",
            "b_sp_0",
            "b_sp_1",
            "b_sp_2",
            "b_sp_3",
        ],
    )

    splt.set_output(transform="pandas")
    X_transformed = splt.transform(X_nan)
    feature_names = splt.get_feature_names_out(["a", "b"])
    assert_array_equal(
        feature_names,
        [
            "a_sp_0",
            "a_sp_1",
            "a_sp_2",
            "a_sp_3",
            "b_sp_0",
            "b_sp_1",
            "b_sp_2",
            "b_sp_3",
        ],
    )

Everything behaves as it should, I believe. But also maybe I didn't understand what you exactly ran into.

ogrisel · Mar 15, 2024

The get_feature_names problem only happens when appending extra output features for the missing indicators (they would then need to be named in that case).

EDIT: I will try to answer your other questions/comments early enough next week.

ogrisel · Mar 15, 2024

About handle_missing="constant", I really prefer handle_missing="zero" or handle_missing="zeros" or handle_missing="ignore" to either convey that missing values are encoded as zero spline feature values or are encoded in a value such that a simple downstream linear model would basically "ignore" them.

StefanieSenger · Apr 18, 2024

Hey @ogrisel, can you give me some feedback?

My current understanding is that if we introduce new 0-values in X_transformed (due to nan values in X), then we also expect different stats for the transformer compared to when no nan values are present.

This would mean, that we expect (and test for)

a different output format in case of nans (I have already written the tests like this)
that the B-splines won't sum up to one * n_features (that I need to define for the test)

StefanieSenger · May 26, 2024

Ah, I just stumbled over Imputation for missing values from the docs. I think we should include "allow_nan" in _more_tags() here, correct?

lorentzenchr · May 27, 2024

Can you ping me once the a reviewer approved?

ogrisel

@StefanieSenger I finally took the time to take a deeper look at some of the remaining open discussion points and failing tests.

doc/whats_new/v1.6.rst

sklearn/preprocessing/_polynomial.py

sklearn/preprocessing/tests/test_polynomial.py

sklearn/preprocessing/_polynomial.py

ogrisel · Apr 18, 2025

I have the feeling that this PR is nearly ready to get in, so I milestoned it for 1.7 but no problem if not ready in time.

StefanieSenger · Apr 18, 2025

Hi, @ogrisel. Thanks for revisiting this PR and for the milestone assignment for version 1.7.

For context, I had kept this PR updated, made it pass all tests and ensured it was merge-ready, but after several months without much engagement from the team, I shifted my attention to other tasks, as the priority wasn’t clear.

Now I'm currently on vacation and will take this back up on my return. I look forward to finalizing it together.

…error messages

StefanieSenger · May 15, 2025

Hi @ogrisel, I have worked on what you had proposed and the tests now all pass.
Do you want to have a look?

StefanieSenger · May 16, 2025

sklearn/preprocessing/_polynomial.py

+
+    def __sklearn_tags__(self):
+        tags = super().__sklearn_tags__()
+        tags.input_tags.allow_nan = self.handle_missing == "zeros"


This way of defining a tag for nan support is per instance, which I think is most user-friendly and is also needed to pass all the common tests. With tags.input_tags.allow_nan = True (or False) either one fails, so we cannot do that.

However, doc/sphinxext/allow_nan_estimators.py that allows to automatically generate a list of estimators is doing it with the default parameter settings (almost as if the tags were defined at class level).
I think I would like to work on a follow-up PR to fix the generation and take cases like this into account. The question would be how large to span that. What do you think, @adrinjalali?

It's a bit sad indeed. But I think we can live with the fact that this list only reflects the default behavior, and that some estimators can be made to accept nans with specific hyper-parameters.

ogrisel

LGTM. Thank you very much for the final push @StefanieSenger!

ogrisel · May 16, 2025

cc @lorentzenchr for a second review.

StefanieSenger added 2 commits December 20, 2023 13:32

added indicator

4255470

added splines full of 0 for nan input in transform

812ae00

github-actions bot added the module:preprocessing label Jan 2, 2024

StefanieSenger and others added 2 commits January 8, 2024 11:39

Merge branch 'main' into nan_SplineTransformer

0ad4f5e

fixed extrapolation='error' related error

f0f7d5e

StefanieSenger changed the title ~~ENH Adds support for np.nan values in SplineTransformer~~ ENH Add support for np.nan values in SplineTransformer Jan 8, 2024

StefanieSenger added 4 commits January 9, 2024 14:14

fixed _get_base_knot_positions so spline parameters are build correctly

a38ee69

exclude knots='quantile' from camparison check

4c9a867

fixed issues from test_common

f7f7ca4

little things

4bbdd4a

ogrisel mentioned this pull request Feb 28, 2024

Handle np.nan / missing values in SplineTransformer #26793

Open

ogrisel reviewed Feb 28, 2024

View reviewed changes

ogrisel mentioned this pull request Feb 28, 2024

Implement SplineTransformer.inverse_transform #28551

Open

Merge branch 'main' into nan_SplineTransformer

092eca0

ogrisel reviewed Mar 1, 2024

View reviewed changes

StefanieSenger and others added 3 commits March 13, 2024 16:22

calculate indicator based on X passed into transform

5165c6b

Merge branch 'main' into nan_SplineTransformer

1a7d134

add and adjust tests after review

708ae53

StefanieSenger commented Mar 15, 2024

View reviewed changes

sklearn/preprocessing/_polynomial.py Show resolved Hide resolved

sklearn/preprocessing/_polynomial.py Outdated Show resolved Hide resolved

sklearn/preprocessing/tests/test_polynomial.py Show resolved Hide resolved

sklearn/preprocessing/tests/test_polynomial.py Outdated Show resolved Hide resolved

wording and typo

1dc3b3c

Merge branch 'main' into nan_SplineTransformer

dce1620

adrinjalali mentioned this pull request Apr 15, 2024

WIP KBinsDiscretizer supports NaN as input values #19928

Open

Merge branch 'main' into nan_SplineTransformer

63ea8aa

StefanieSenger marked this pull request as ready for review April 18, 2024 13:51

adrinjalali mentioned this pull request Jun 5, 2024

sklearn.neighbors.NearestNeighbors allow processing nan values #29085

Closed

StefanieSenger and others added 4 commits October 9, 2024 10:57

Merge branch 'main' into nan_SplineTransformer

bd867af

fix linting

55feb20

fix FutureWarning for force_all_finite

407165a

Merge branch 'main' into nan_SplineTransformer

71b09ef

StefanieSenger closed this Mar 6, 2025

StefanieSenger reopened this Mar 6, 2025

ogrisel added 2 commits April 18, 2025 15:05

Merge branch 'main' into nan_SplineTransformer

da8fcdc

Handle all-nan columns as constant columns

b6239fb

ogrisel reviewed Apr 18, 2025

View reviewed changes

ogrisel added this to the 1.7 milestone Apr 18, 2025

StefanieSenger and others added 8 commits May 6, 2025 11:30

update changelog

f6ab8f8

improve comments

3f47364

remove erroring behavior for nan and sample_weight and improve other …

656685b

…error messages

fix input_tags for nans

6571a12

add test for sample_weight and nan value handling

a41deb1

fix exception handling for nan input

6abb0af

fix shape mismatch in CI due to sparse-dense conversion

038ffe8

Merge branch 'main' into nan_SplineTransformer

fe27494

lucyleeow mentioned this pull request May 16, 2025

Inconsistent median/quantile behaviour now _weighted_percentile ignores NaNs #31367

Closed

StefanieSenger commented May 16, 2025

View reviewed changes

ogrisel modified the milestones: 1.7, 1.8 May 16, 2025

ogrisel approved these changes May 16, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH Add support for np.nan values in SplineTransformer #28043

ENH Add support for np.nan values in SplineTransformer #28043

StefanieSenger commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

ogrisel left a comment •

edited

Loading

ogrisel commented Feb 29, 2024

ogrisel left a comment

ogrisel Mar 1, 2024

StefanieSenger left a comment

StefanieSenger commented Mar 15, 2024

ogrisel commented Mar 15, 2024 •

edited

Loading

ogrisel commented Mar 15, 2024

StefanieSenger commented Apr 18, 2024 •

edited

Loading

StefanieSenger commented May 26, 2024 •

edited

Loading

lorentzenchr commented May 27, 2024

ogrisel left a comment •

edited

Loading

ogrisel commented Apr 18, 2025

StefanieSenger commented Apr 18, 2025

StefanieSenger commented May 15, 2025

StefanieSenger May 16, 2025

ogrisel May 16, 2025

ogrisel left a comment

ogrisel commented May 16, 2025

Search code, repositories, users, issues, pull requests...

ENH Add support for np.nan values in SplineTransformer #28043

Are you sure you want to change the base?

ENH Add support for np.nan values in SplineTransformer #28043

Conversation

StefanieSenger commented Jan 2, 2024 • edited Loading

Reference Issues/PRs

What does this implement/fix? Explain your changes.

github-actions bot commented Jan 2, 2024 • edited Loading

✔️ Linting Passed

ogrisel left a comment • edited Loading

Choose a reason for hiding this comment

ogrisel commented Feb 29, 2024

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel Mar 1, 2024

Choose a reason for hiding this comment

StefanieSenger left a comment

Choose a reason for hiding this comment

StefanieSenger commented Mar 15, 2024

ogrisel commented Mar 15, 2024 • edited Loading

ogrisel commented Mar 15, 2024

StefanieSenger commented Apr 18, 2024 • edited Loading

StefanieSenger commented May 26, 2024 • edited Loading

lorentzenchr commented May 27, 2024

ogrisel left a comment • edited Loading

Choose a reason for hiding this comment

ogrisel commented Apr 18, 2025

StefanieSenger commented Apr 18, 2025

StefanieSenger commented May 15, 2025

StefanieSenger May 16, 2025

Choose a reason for hiding this comment

ogrisel May 16, 2025

Choose a reason for hiding this comment

ogrisel left a comment

Choose a reason for hiding this comment

ogrisel commented May 16, 2025

StefanieSenger commented Jan 2, 2024 •

edited

Loading

github-actions bot commented Jan 2, 2024 •

edited

Loading

ogrisel left a comment •

edited

Loading

ogrisel commented Mar 15, 2024 •

edited

Loading

StefanieSenger commented Apr 18, 2024 •

edited

Loading

StefanieSenger commented May 26, 2024 •

edited

Loading

ogrisel left a comment •

edited

Loading