[MRG] Avoid uncessary copies in sklearn.preprocessing #13987
Merged
Conversation
NicolasHug
approved these changes
May 30, 2019
LGTM.
Looks like in these specific cases inplace would have been a more descriptive parameter name than copy, and might have prevented this.
thomasjpfan
approved these changes
May 30, 2019
LGTM
|
Does this need a whats_new entry as an enhancement or a bug fix? |
|
Thanks for the reviews! Added a what's new. |
|
|
9661a64
into
scikit-learn:master
16 checks passed
16 checks passed
|
Thank you @rth! |
koenvandevelde
added a commit
to koenvandevelde/scikit-learn
that referenced
this issue
Jul 12, 2019
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Partially addresses #13986
This removes the
copy=Truein the fit method ofStandardScaler,MinMaxScaler,MaxAbsScaler,RobustScalerwhere it is typically not necessary to compute the scaling factors.In practice, this makes
StandardScaler().fit_transform10%-20% faster on the few examples I have tried.If that copy was necessary and this mistakenly removed it
check_transformer_general(.., readonly_memmap=True)would fail in common tests.The text was updated successfully, but these errors were encountered: