-
-
Notifications
You must be signed in to change notification settings - Fork 26k
[MRG+1] y not needed in transform in Birch #9066
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
It's also the case for HashingVectorizer, PCA and all classes inheriting from BaseRandomProjection and SparseCodingMixin . Wasn't that about compatibility with the transformers API? |
ok this is taken care of now. thx @rth no transform should not have a y. It's not part of the API. see for example : https://github.com/scikit-learn/scikit-learn/blob/master/sklearn/pipeline.py#L315 |
all green :) |
LGTM. +1 for merge |
I think last time we saw this we said "oh let's wait on the API discussions" but I think we can remove this now. |
1 similar comment
I think last time we saw this we said "oh let's wait on the API discussions" but I think we can remove this now. |
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
* y not needed in transform * y not needed in transform * more
Birch had a y in transform method signature