Fixed #1654: Fix mknfold using new StratifiedKFold API #1660
Conversation
|
Java flaky test: https://travis-ci.org/dmlc/xgboost/jobs/167141727 |
|
I am going to merge this in one. wait @CodingCat to fix the java testcase |
|
Hmmm....this has appeared for multiple times, I will deprecate this test temporarily and figure out the reason later |
|
this commit makes it impossible to pass K Fold as an nfold argument (see comment in code). Please revert or add possibility to not shuffle (PR #1459) |
|
Could you elaborate? I don't fully understand. There's no comment in the code either |
|
line 235 you added the check to see if the folds are an instance of list. |
| @@ -232,14 +232,12 @@ def mknfold(dall, nfold, param, seed, evals=(), fpreproc=None, stratified=False, | |||
| randidx = np.random.permutation(dall.num_row()) | |||
| kstep = int(len(randidx) / nfold) | |||
| idset = [randidx[(i * kstep): min(len(randidx), (i + 1) * kstep)] for i in range(nfold)] | |||
| elif folds is not None: | |||
| elif folds is not None and isinstance(folds, list): | |||
There was a problem hiding this comment.
this check blocks off KFold usage
There was a problem hiding this comment.
I commented on your PR. Could you address this in your PR too?
There was a problem hiding this comment.
Sure, thanks for the quick reply
Fixed #1654 @tqchen @CodingCat