8000 RFC binary operators on estimators · Issue #7608 · scikit-learn/scikit-learn · GitHub
[go: up one dir, main page]
Skip to content

RFC binary operators on estimators #7608

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.

Sign up for GitHub

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

Jump to bottom
Open
amueller opened this issue Oct 7, 2016 · 5 comments
Open

RFC binary operators on estimators #7608

amueller opened this issue Oct 7, 2016 · 5 comments
Labels
API Needs Decision Requires decision

Comments

@amueller
Copy link
Member
amueller commented Oct 7, 2016

So maybe this is not that bad an idea?

https://gist.github.com/amueller/643f812a275a9e0c75048aab6988a92c

@jnothman can you maybe point me to what you meant?
@jnothman
Copy link
Member
jnothman commented Oct 8, 2016

In what way is it not a bad idea? :P

The concern I commented on is that

GridSearchCV(MyEst(), param_dict={'foo': bar})

then with a pipeline needs adjustment:

GridSearchCV(Transf() >> MyEst(), param_dict={'MyEst__foo': bar})

This adjustment is much less obvious with operators...

Why __mul__ not __add__?

@amueller
Copy link
Member Author
amueller commented Oct 8, 2016

Should probably be __add__

I would argue that just shows how bad our GridSearchCV interface is ;) I suggested at some point attaching the parameters to the estimator, that would get rid of the problem.

GridSearchCV(Transf() >> MyEst().search_over({'foo':bar}))

But I guess that remains a pipe-dream ;)

@amueller
Copy link
Member Author
amueller commented Oct 8, 2016

Oh you created an issue for that even #5082 ... it's been a while

@jnothman
Copy link
Member
jnothman commented Oct 8, 2016

I wonder whether we should have an Epic tag for such things, perhaps instead of SLEPS...

@amueller
Copy link
Member Author
amueller commented Oct 8, 2016

well we now each have a list of pets ;)

@amueller amueller mentioned this issue Oct 18, 2016
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
API Needs Decision Requires decision
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
4 participants
@jnothman @amueller @thomasjpfan @lorentzenchr
0