-
-
Notifications
You must be signed in to change notification settings - Fork 25.8k
[MRG+1] TST: estimator_checks: skip transform test when not implemented #6510
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
Conversation
1cf68bc
to
f09a1bd
Compare
we have models that don't have transform, like TSNE. I think they don't inherit from |
In that case, should we remove similar checks that are already in there? e.g. https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/utils/estimator_checks.py#L578 |
hm ok that is indeed inconsistent. I'm just a bit confused that we are not running into any troubles in master. |
I believe it's because we're explicitly leaving out by name the transformers that don't have a This, of course, doesn't help third-party estimators that we'd like to exclude as well. |
All the exceptions by name have to go! On 9 March 2016 at 10:32, Jake Vanderplas notifications@github.com wrote:
|
This solution LGTM, but I would like a comment that "may have fit_transform but not transform" |
TSNE is therefore not a transformer?
On 20 Jun 2017 5:20 am, "Andreas Mueller" <notifications@github.com> wrote:
I still think that a transformer needs a transform method. Unfortunately
it's unclear to me now what exception @jakevdp <https://github.com/jakevdp>
pointed to because line-numbers move :-/
All exceptions by name are gone in #8022
<#8022>
—
You are receiving this because you commented.
Reply to this email directly, view it on GitHub
<#6510 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/AAEz6-0HB_m5hCfqt8-em__KUhrmLyzNks5sFsoDgaJpZM4HsNm_>
.
|
@jnothman yes TSNE is not a transformer ;) |
Closing this issue from 2016 due to reviewer disagreement about this being the right solution. Currently, a better way of achieving this would probably involve estimator tags. |
Estimator checks fail when a transformer only implements
fit_transform
.