Skip to content

DOC Add links to preprocessing examples in docstrings and userguide #26877

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

Conversation

StefanieSenger
Copy link
Contributor

@StefanieSenger StefanieSenger commented Jul 21, 2023

This PR suggests to add links to the examples from the Preprocessing section to the docstrings of the respective classes and functions.

@StefanieSenger StefanieSenger changed the title links to preprocessing examples added DOC Add links to preprocessing examples in docstrings and userguide Jul 21, 2023
@github-actions
Copy link

github-actions bot commented Jul 21, 2023

✔️ Linting Passed

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

Generated for commit: beb8ca8. Link to the linter CI: here

Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @StefanieSenger. Here is a first batch of comments.

Copy link
Member

@ArturoAmorQ ArturoAmorQ left a comment

Choose a reason for hiding this comment

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

Apart from a bit of wording, LGTM :) Thanks again @StefanieSenger!

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Copy link
Member

@adrinjalali adrinjalali left a comment

Choose a reason for hiding this comment

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

Left a few comments, it's hard to review since this PR is rather large and touching many examples, it's easier if PRs have a smaller scope.

@adrinjalali
Copy link
Member

@StefanieSenger
Copy link
Contributor Author

Resolved the CI issues, thank you @adrinjalali

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.

Overall, LGTM, thanks for the PR. In addition to @adrinjalali's remarks here are a few more.

@@ -291,6 +290,10 @@ class MinMaxScaler(OneToOneFeatureMixin, TransformerMixin, BaseEstimator):
This transformation is often used as an alternative to zero mean,
unit variance scaling.

MinMaxScaler doesn't reduce the effect of outliers; it only linearily
scales them down. For an example visualization, refer to :ref:`Compare
Copy link
Member

Choose a reason for hiding this comment

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

This statement holds for all scalers (StandardScaler, RobustScaler, MaxAbsScaler and MinMaxScaler). What is different is that the scale value found by RobustScaler is not sensitive to the presence of a few large marginal outliers while it is for StandardScaler and even more so for MinMaxScaler and MaxAbsScaler.

Copy link
Contributor Author

@StefanieSenger StefanieSenger Aug 23, 2023

Choose a reason for hiding this comment

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

Yes, I see. To express how the MinMaxScaler differs from the other scalers concerning outliers, I have tried to come up with a new wording:

    `MinMaxScaler` doesn't reduce the effect of outliers, but it linearily
    scales them down into a fixed range, where the largest occuring data point
    corresponds to the maximum value and the smallest one corresponds to the
    minimum value.

What do you think?

StefanieSenger and others added 7 commits August 23, 2023 21:33
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
…er/scikit-learn into link_examples_preprocessing
@adrinjalali adrinjalali merged commit 8ccaf0d into scikit-learn:main Aug 24, 2023
akaashpatelmns pushed a commit to akaashp2000/scikit-learn that referenced this pull request Aug 25, 2023
…cikit-learn#26877)

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Aug 29, 2023
…cikit-learn#26877)

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Sep 18, 2023
…cikit-learn#26877)

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
jeremiedbb pushed a commit that referenced this pull request Sep 20, 2023
…26877)

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
REDVM pushed a commit to REDVM/scikit-learn that referenced this pull request Nov 16, 2023
…cikit-learn#26877)

Co-authored-by: Arturo Amor <86408019+ArturoAmorQ@users.noreply.github.com>
Co-authored-by: Adrin Jalali <adrin.jalali@gmail.com>
Co-authored-by: Olivier Grisel <olivier.grisel@ensta.org>
@StefanieSenger StefanieSenger deleted the link_examples_preprocessing branch April 18, 2024 11:00
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.

4 participants