Skip to content
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

RFC binary operators on estimators #7608

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

Comments

@amueller
Copy link
Member

@amueller 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

This comment has been minimized.

Copy link
Member

@jnothman 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

This comment has been minimized.

Copy link
Member Author

@amueller 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

This comment has been minimized.

Copy link
Member Author

@amueller amueller commented Oct 8, 2016

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

@jnothman

This comment has been minimized.

Copy link
Member

@jnothman jnothman commented Oct 8, 2016

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

@amueller

This comment has been minimized.

Copy link
Member Author

@amueller amueller commented Oct 8, 2016

well we now each have a list of pets ;)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
2 participants
You can’t perform that action at this time.