Join GitHub today
GitHub is home to over 40 million developers working together to host and review code, manage projects, and build software together.
Sign upAPI don't default get_params output to None in the future #14464
Conversation
| @@ -127,9 +127,9 @@ class Pipeline(_BaseComposition): | |||
|
|
|||
| def __init__(self, steps, memory=None, verbose=False): | |||
| self.steps = steps | |||
| self._validate_steps() | |||
This comment has been minimized.
This comment has been minimized.
| pass | ||
|
|
||
| def fit(self, X, y=None): | ||
| return self |
This comment has been minimized.
This comment has been minimized.
| return X | ||
|
|
||
| def diag(self, X): | ||
| return np.ones(X.shape[0]) |
This comment has been minimized.
This comment has been minimized.
|
lgtm |
This comment has been minimized.
This comment has been minimized.
|
Even if affected users will see the warning maybe it's also worth adding a what's new entry? |
jnothman
merged commit 0279108
into
scikit-learn:master
16 of 17 checks passed
16 of 17 checks passed
scikit-learn.scikit-learn (Linux pylatest_conda_mkl_pandas)
Linux pylatest_conda_mkl_pandas succeeded
Details
scikit-learn.scikit-learn (Linux32 py35_ubuntu_atlas_32bit)
Linux32 py35_ubuntu_atlas_32bit succeeded
Details
scikit-learn.scikit-learn (Windows py35_pip_openblas_32bit)
Windows py35_pip_openblas_32bit succeeded
Details
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.
jnothman commentedJul 25, 2019
Reference Issues/PRs
Fixes #14461
What does this implement/fix? Explain your changes.
Previously, the default implementation of
get_paramswould usegetattrto retrieve a parameter value from an estimator, but would default to retrieve None if an AttributeError would otherwise have been raised.This can lead to surprising behaviour upon
clonewhen__init__is not correctly defined, as shown in #14461.I propose that we raise an AttributeError when a param cannot be retrieved by
getattr. This PR deprecates returning None, and raises a FutureWarning.Any other comments?
TODO: add tests