Skip to content

[MRG+2] FEA Add SplineTransformer #18368

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 79 commits into from
Jan 24, 2021
Merged

Conversation

lorentzenchr
Copy link
Member

@lorentzenchr lorentzenchr commented Sep 9, 2020

Reference Issues/PRs

Closes #17027.

What does this implement/fix? Explain your changes.

This PR adds a new transformer preprocessing.SplineTransformer that creates a B-spline basis for each feature. This is very convenient for non-linear continuous feature effects in linear models.

Any other comments? Edited

As an example, polynomial interpolation is extended and explains the SplineTransformer a bit.
Possible other examples to put it in are preprocessing plot discretization or maybe adaboost regression.
They could also be used in some more examples to improve the linear models.

Not implemented are:

  • Support for sparse output. This makes a lot of sense as splines have local support => only degree number of output columns are non-zero per row (per input feature).
  • Periodic extrapolation as in scipy.interpolate.BSpline(..., extrapolation='periodic').

@lorentzenchr
Copy link
Member Author

@eickenberg Is this good enough for your cheering?:smile:

@lorentzenchr lorentzenchr changed the title [WIP] Add SplineTransformer [MRG] Add SplineTransformer Sep 14, 2020
@lorentzenchr
Copy link
Member Author

I think it has reached readiness for merge.

@lorentzenchr lorentzenchr changed the title [MRG] Add SplineTransformer [MRG+1] Add SplineTransformer Jan 14, 2021
Copy link
Member

@glemaitre glemaitre left a comment

Choose a reason for hiding this comment

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

LGTM Maybe just a last approval of @jnothman @thomasjpfan before merging

Base automatically changed from master to main January 22, 2021 10:53
@lorentzenchr lorentzenchr changed the title [MRG+1] Add SplineTransformer [MRG+2] FEA Add SplineTransformer Jan 23, 2021
@jnothman
Copy link
Member

LGTM Maybe just a last approval of @jnothman @thomasjpfan before merging

Why? Do you have specific concerns, or want to highlight changes since @agramfort's approval?

@glemaitre
Copy link
Member

I wanted just to make sure that you were happy with the changes to address #18368 (comment)

I ping @thomasjpfan by mistake.

@jnothman
Copy link
Member

jnothman commented Jan 23, 2021 via email

@glemaitre glemaitre self-assigned this Jan 24, 2021
@glemaitre
Copy link
Member

I pushed a small change to cross-reference adding a See Also section. I will check the rendering and merge the PR if everything is looking good (and fix it if I mess up).

@glemaitre glemaitre merged commit 27f1c73 into scikit-learn:main Jan 24, 2021
@glemaitre
Copy link
Member

Thanks @lorentzenchr

@agramfort
Copy link
Member

agramfort commented Jan 24, 2021 via email

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Spline Transformer
6 participants