The Wayback Machine - https://web.archive.org/web/20220218115010/https://github.com/scikit-learn/scikit-learn/pull/18368
Skip to content
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

[MRG+2] FEA Add SplineTransformer #18368

Merged
merged 79 commits into from Jan 24, 2021
Merged

[MRG+2] FEA Add SplineTransformer #18368

merged 79 commits into from Jan 24, 2021

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

@lorentzenchr lorentzenchr commented Sep 9, 2020

@eickenberg Is this good enough for your cheering?😄

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

@lorentzenchr lorentzenchr commented Jan 14, 2021

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
Contributor

@glemaitre glemaitre left a comment

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

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

@jnothman jnothman commented Jan 23, 2021

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
Contributor

@glemaitre glemaitre commented Jan 23, 2021

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 jnothman commented Jan 23, 2021

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

@glemaitre glemaitre commented Jan 24, 2021

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
27 checks passed
@glemaitre
Copy link
Contributor

@glemaitre glemaitre commented Jan 24, 2021

Thanks @lorentzenchr

@agramfort
Copy link
Member

@agramfort agramfort commented Jan 24, 2021

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

Successfully merging this pull request may close these issues.

6 participants