{"action":"created","issue":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/7604","repository_url":"https://api.github.com/repos/scikit-learn/scikit-learn","labels_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/7604/labels{/name}","comments_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/7604/comments","events_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/7604/events","html_url":"https://github.com/scikit-learn/scikit-learn/pull/7604","id":181767010,"number":7604,"title":"[MRG + 1] Fix min_samples_split and min_samples_leaf validation for float vs int","user":{"login":"nelson-liu","id":7272031,"avatar_url":"https://avatars.githubusercontent.com/u/7272031?v=3","gravatar_id":"","url":"https://api.github.com/users/nelson-liu","html_url":"https://github.com/nelson-liu","followers_url":"https://api.github.com/users/nelson-liu/followers","following_url":"https://api.github.com/users/nelson-liu/following{/other_user}","gists_url":"https://api.github.com/users/nelson-liu/gists{/gist_id}","starred_url":"https://api.github.com/users/nelson-liu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nelson-liu/subscriptions","organizations_url":"https://api.github.com/users/nelson-liu/orgs","repos_url":"https://api.github.com/users/nelson-liu/repos","events_url":"https://api.github.com/users/nelson-liu/events{/privacy}","received_events_url":"https://api.github.com/users/nelson-liu/received_events","type":"User","site_admin":false},"labels":[],"state":"open","locked":false,"assignee":null,"assignees":[],"milestone":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/milestones/22","html_url":"https://github.com/scikit-learn/scikit-learn/milestones/0.18.1","labels_url":"https://api.github.com/repos/scikit-learn/scikit-learn/milestones/22/labels","id":2046567,"number":22,"title":"0.18.1","description":"Urgent fixes to bugs and web site issues.","creator":{"login":"jnothman","id":78827,"avatar_url":"https://avatars.githubusercontent.com/u/78827?v=3","gravatar_id":"","url":"https://api.github.com/users/jnothman","html_url":"https://github.com/jnothman","followers_url":"https://api.github.com/users/jnothman/followers","following_url":"https://api.github.com/users/jnothman/following{/other_user}","gists_url":"https://api.github.com/users/jnothman/gists{/gist_id}","starred_url":"https://api.github.com/users/jnothman/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/jnothman/subscriptions","organizations_url":"https://api.github.com/users/jnothman/orgs","repos_url":"https://api.github.com/users/jnothman/repos","events_url":"https://api.github.com/users/jnothman/events{/privacy}","received_events_url":"https://api.github.com/users/jnothman/received_events","type":"User","site_admin":false},"open_issues":14,"closed_issues":4,"state":"open","created_at":"2016-10-04T11:55:58Z","updated_at":"2016-10-08T03:01:05Z","due_on":null,"closed_at":null},"comments":4,"created_at":"2016-10-07T21:02:31Z","updated_at":"2016-10-08T03:06:18Z","closed_at":null,"pull_request":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/pulls/7604","html_url":"https://github.com/scikit-learn/scikit-learn/pull/7604","diff_url":"https://github.com/scikit-learn/scikit-learn/pull/7604.diff","patch_url":"https://github.com/scikit-learn/scikit-learn/pull/7604.patch"},"body":"#### Reference Issue\r\n<!-- Example: Fixes #1234 -->\r\nFixes #7603 \r\n\r\n#### What does this implement/fix? Explain your changes.\r\nThe float and int values of `min_samples_split` and `min_samples_leaf` were not being properly checked, because large float values could pass the int check. I fixed the checks and moved them to a better location before the parameters are used. I also fixed the wording of the warning and its return value as it was improperly reporting `min_samples_split` and `min_samples_leaf` when it should have been reporting the value of `self.min_samples_split` and `self.min_samples_leaf`. Lastly, I added a non-regression test for this to the test suite.\r\n\r\n\r\n#### Any other comments?\r\n@mgoldwasser mentioned:\r\n>  In theory, it would be useless to set min_samples_split to 1.0 or more, or min_samples_leaf to 0.5 or more\r\n\r\nThis is indeed true, and we do check for this. However, we don't validate that `min_samples_split` and `min_samples_leaf` are less than (1.0 * n_samples) and (0.5 * n_samples) when ints are provided; should we add this?"},"comment":{"url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/comments/252398840","html_url":"https://github.com/scikit-learn/scikit-learn/pull/7604#issuecomment-252398840","issue_url":"https://api.github.com/repos/scikit-learn/scikit-learn/issues/7604","id":252398840,"user":{"login":"nelson-liu","id":7272031,"avatar_url":"https://avatars.githubusercontent.com/u/7272031?v=3","gravatar_id":"","url":"https://api.github.com/users/nelson-liu","html_url":"https://github.com/nelson-liu","followers_url":"https://api.github.com/users/nelson-liu/followers","following_url":"https://api.github.com/users/nelson-liu/following{/other_user}","gists_url":"https://api.github.com/users/nelson-liu/gists{/gist_id}","starred_url":"https://api.github.com/users/nelson-liu/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/nelson-liu/subscriptions","organizations_url":"https://api.github.com/users/nelson-liu/orgs","repos_url":"https://api.github.com/users/nelson-liu/repos","events_url":"https://api.github.com/users/nelson-liu/events{/privacy}","received_events_url":"https://api.github.com/users/nelson-liu/received_events","type":"User","site_admin":false},"created_at":"2016-10-08T03:06:18Z","updated_at":"2016-10-08T03:06:18Z","body":"sorry if this is a dumb question, but what exactly does it mean to backport? do you just merge the changes into the 0.18 branch? Would this affect the files that were already released as \"0.18\" in the sense that if I were to `conda install scikit-learn` after you backported I would get the new changes?"}}
{"actor":{"display_login":"nelson-liu"}}