-
Notifications
You must be signed in to change notification settings - Fork 7
Clean docstrings of algo fit #329
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
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.
Thanks @caglayantuna !
LGTM, just a couple minor suggestions.
@@ -198,64 +202,15 @@ def _initialize_algo(self, model: AbstractModel, dataset: Dataset) -> State: | |||
|
|||
return state | |||
|
|||
def _terminate_algo( |
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.
I thought we deleted this already, didn't we ?
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.
I opened a second PR and closed previous one when I am working on it. It seems that I forgot to delete it in the second PR. So, it is time remove it because it is useless
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.
OK, thanks for the explanation.
apply suggestions Co-authored-by: Gensollen <nicolas.gensollen@gmail.com>
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, thanks @caglayantuna !
@@ -198,64 +202,15 @@ def _initialize_algo(self, model: AbstractModel, dataset: Dataset) -> State: | |||
|
|||
return state | |||
|
|||
def _terminate_algo( |
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.
OK, thanks for the explanation.
This PR updates docstrings of
algo/fit
files as it is mentioned in issue #122. I checked the doc locally and it seems ok to me. Also, I corrected minor things indag.py
file related to docstrings.