-
Notifications
You must be signed in to change notification settings - Fork 16
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
ENH add monitor keyword to the autoencoder #58
Conversation
@samcarlos This one is good to go. |
@@ -516,13 +516,22 @@ def _build_sigmoid_and_softmax_indices(self): | |||
raise ValueError("Sigmoid indices and softmax indices cannot" | |||
" overlap!") | |||
|
|||
def partial_fit(self, X, y=None): | |||
def partial_fit(self, X, y=None, monitor=None, **kwargs): |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why do you pass kwargs here? Is there anything in partial_fit that takes that as an input?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thus is part of the sklearn API. See here http://scikit-learn.org/stable/modules/generated/sklearn.base.TransformerMixin.html#sklearn.base.TransformerMixin.fit_transform
@samcarlos This is fixed! Back to you! |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM
LGTM
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
LGTM
LGTM
LGTM
This PR adds the monitor keyword to the autoencoder.
Closes #20