FIX Shuffle each class's samples with different random_state in StratifiedKFold (scikit-learn#13124)

qinhanmin2014 · jnothman · commit d2b405ce5035 · 2019-07-23T13:03:40.000+10:00
* Enable StratifiedKFold to produce different splits

* what's new

* redundant statement

* update what's new

* redundant comment

* add a test

* move what's new entry

* review comment

* review comment
diff --git a/doc/whats_new/v0.20.rst b/doc/whats_new/v0.20.rst
@@ -7,6 +7,26 @@
 Version 0.20.3
 ==============
 
+**July 29, 2019**
+
+This is a bug-fix release with some bug fixes applied to version 0.20.3.
+
+Changelog
+---------
+
+:mod:`sklearn.model_selection`
+..............................
+
+- |Fix| Fixed a bug where :class:`model_selection.StratifiedKFold`
+  shuffles each class's samples with the same ``random_state``,
+  making ``shuffle=True`` ineffective.
+  :issue:`13124` by :user:`Hanmin Qin <qinhanmin2014>`.
+
+ .. _changes_0_20_3:
+
+Version 0.20.3
+==============
+
 **March 1, 2019**
 
 This is a bug-fix release with some minor documentation improvements and
diff --git a/sklearn/model_selection/_split.py b/sklearn/model_selection/_split.py
@@ -583,8 +583,7 @@ class StratifiedKFold(_BaseKFold):
             ``n_splits`` default value will change from 3 to 5 in v0.22.
 
     shuffle : boolean, optional
-        Whether to shuffle each stratification of the data before splitting
-        into batches.
+        Whether to shuffle each class's samples before splitting into batches.
 
     random_state : int, RandomState instance or None, optional, default=None
         If int, random_state is the seed used by the random number generator;
@@ -626,7 +625,7 @@ def __init__(self, n_splits='warn', shuffle=False, random_state=None):
         super(StratifiedKFold, self).__init__(n_splits, shuffle, random_state)
 
     def _make_test_folds(self, X, y=None):
-        rng = self.random_state
+        rng = check_random_state(self.random_state)
         y = np.asarray(y)
         type_of_target_y = type_of_target(y)
         allowed_target_types = ('binary', 'multiclass')
diff --git a/sklearn/model_selection/tests/test_split.py b/sklearn/model_selection/tests/test_split.py
@@ -504,6 +504,17 @@ def test_shuffle_stratifiedkfold():
         assert_not_equal(set(test0), set(test1))
     check_cv_coverage(kf0, X_40, y, groups=None, expected_n_splits=5)
 
+    # Ensure that we shuffle each class's samples with different
+    # random_state in StratifiedKFold
+    # See https://github.com/scikit-learn/scikit-learn/pull/13124
+    X = np.arange(10)
+    y = [0] * 5 + [1] * 5
+    kf1 = StratifiedKFold(5, shuffle=True, random_state=0)
+    kf2 = StratifiedKFold(5, shuffle=True, random_state=1)
+    test_set1 = sorted([tuple(s[1]) for s in kf1.split(X, y)])
+    test_set2 = sorted([tuple(s[1]) for s in kf2.split(X, y)])
+    assert test_set1 != test_set2
+
 
 def test_kfold_can_detect_dependent_samples_on_digits():  # see #2372
     # The digits samples are dependent: they are apparently grouped by authors
