Skip to content

DOC linked SplineTransformer to time-related feature engineering example #26971

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 3 commits into from
Dec 4, 2023
Merged

DOC linked SplineTransformer to time-related feature engineering example #26971

merged 3 commits into from
Dec 4, 2023

Conversation

scaja
Copy link
Contributor

@scaja scaja commented Aug 1, 2023 •
edited
Loading

Reference Issues/PRs

issue #26927

What does this implement/fix? Explain your changes.

This PR linked SplineTransformer to time-related feature engineering example

Related Example files:

plot_cyclical_feature_engineering.py

@github-actions
Copy link

github-actions bot commented Aug 1, 2023 •
edited
Loading

✔️ Linting Passed

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

Generated for commit: 8678ea4. Link to the linter CI: here

@scaja scaja changed the title linked SplineTransformer to time-related feature engineering example DOC linked SplineTransformer to time-related feature engineering example Aug 1, 2023
adrinjalali
adrinjalali approved these changes Aug 3, 2023
View reviewed changes
sklearn/preprocessing/_polynomial.py
Comment on lines +587 to +588
In order to learn more about the SplineTransformer class go to:
:ref:`sphx_glr_auto_examples_applications_plot_cyclical_feature_engineering.py`
Copy link
Member

Choose a reason for hiding this comment

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

This is a good example, and could be mentioned in more places since it's more on the realistic side of things.

I'd suggest having a link in ColumnTransformer, Pipeline, and TimeSeriesSplit as well.

But the description could be more informative. Something like:

Fore a more comprehensive example on a time series data est, see ...

WDYT @glemaitre

Copy link
Member

Choose a reason for hiding this comment

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

Definitely for TimeSeriesSplit. For ColumnTransformer and Pipeline, let's do it as well. We have to be careful in the future that for those two, we don't have too many examples.

@adrinjalali adrinjalali mentioned this pull request Aug 3, 2023
Closed
@adrinjalali adrinjalali added Stalled good first issue Easy with clear instructions to resolve help wanted labels Sep 29, 2023
@glemaitre glemaitre self-requested a review November 1, 2023 12:04
@manthanindane
Copy link

I'm new here and figuring out some things.......Is this issue still open? Is anybody working on this?

@glemaitre
Copy link
Member

@manthanindane The corresponding issue is still open here: #26927

ogrisel
ogrisel approved these changes Nov 20, 2023
View reviewed changes
Copy link
Member

@ogrisel ogrisel left a comment

Choose a reason for hiding this comment

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

+1 for merging this PR as such as it's already a net improvement.

@adrinjalali
Copy link
Member

Added the link for TimeSeriesSplit, leaving the other ones alone. Feel free to merge @ogrisel or @glemaitre

@glemaitre glemaitre enabled auto-merge (squash) December 4, 2023 15:22
@glemaitre glemaitre merged commit f113327 into scikit-learn:main Dec 4, 2023
@Ujjwal29
Copy link

Ujjwal29 commented Apr 3, 2024

How do we figure out which example file reference to be added in which file?

@adrinjalali
Copy link
Member

@Ujjwal29 it's either in the docstrings of related classes (inside the .py files where they're implemented) or in the user guides (the .rst files where the user guides are written)

@ArturoAmorQ ArturoAmorQ mentioned this pull request Jun 6, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@ogrisel ogrisel ogrisel approved these changes

@adrinjalali adrinjalali adrinjalali approved these changes

@glemaitre glemaitre glemaitre approved these changes

Assignees
No one assigned
Labels
Documentation good first issue Easy with clear instructions to resolve help wanted module:preprocessing Stalled
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@scaja @manthanindane @glemaitre @adrinjalali @Ujjwal29 @ogrisel