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

Parameters validation #24

Merged
merged 1 commit into from
Jun 3, 2017
Merged

Parameters validation #24

merged 1 commit into from
Jun 3, 2017

Conversation

StrikerRUS
Copy link
Member

No description provided.

@fukatani fukatani merged commit a33ef43 into RGF-team:master Jun 3, 2017
@fukatani
Copy link
Member

fukatani commented Jun 3, 2017

Hi @StrikerRUS .
Thank you for your PR.

I merged #25, at the same time, it seems that this branch was also merged unintentionally.
But as you can see, changes to the code by PR are not included in the master branch.
Since I don't know how to reopen this PR, could you reopen this?

Sorry for inconvenience.

If you want to open two PR, I recommend always branch from master, and not work on master branch.

@fukatani
Copy link
Member

fukatani commented Jun 3, 2017

I recommend:
elif max_leaf <= 0: rather than elif (max_leaf <= 0):
elif not loss in _LOSSES: rather than elif not (loss in _LOSSES):

In future, I apply partial of PEP8 to this repo.

Also we need new test which input type is checked.
Both valid and invalid type should be input.

@fukatani fukatani mentioned this pull request Jun 3, 2017
@StrikerRUS
Copy link
Member Author

Oh, it's my lack of practice with Git, sorry!
Next time I'll open only one PR at the same time, it'll less painful.
I'll add new test and open new PR, sorry again.

@StrikerRUS StrikerRUS deleted the check_params branch June 3, 2017 09:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants