ENH Allow isotonic reg 2d input with 1 feature (GH15012) #17379

fujiaxiang · 2020-05-29T03:43:15Z

Reference Issues/PRs

What does this implement/fix? Explain your changes.

This PR allows IsotonicRegression to accept 2D inputs in its fit, predict and transform methods.

fujiaxiang · 2020-05-29T03:47:25Z

This PR closes 2 issues as far as I know.

Because #17333 is the direct cause of #15012, I did not mention #17333 in my comments. Hope that's the right thing to do.

fujiaxiang · 2020-05-29T03:52:03Z

btw I tested the fix locally, it does indeed fix #17333. Again because it's essentially the same issue, I did not add any tests specifically testing IsotonicRegression used in an ensemble.

If that test should be added, just let me know!
Cheers!

thomasjpfan

Thank you @fujiaxiang !

sklearn/isotonic.py

sklearn/tests/test_isotonic.py

NicolasHug

Thanks @fujiaxiang , a few comments but looks good in general!

doc/whats_new/v0.24.rst

sklearn/isotonic.py

sklearn/tests/test_isotonic.py

change flatten to reshape(-1) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

change array(list(range(10))) to arange(10) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

simplify test input Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

simplify code Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

change assert_array_equal to assert_allclose Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

fujiaxiang · 2020-06-02T14:13:34Z

@thomasjpfan @NicolasHug thanks for reviewing.
Have made the changes accordingly. Can you review again?

thomasjpfan

Minor comments, otherwise LGTM

doc/whats_new/v0.24.rst

sklearn/tests/test_isotonic.py

reformat whatsnew entry Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

clean up comment in test case Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

fujiaxiang · 2020-06-02T15:31:40Z

@thomasjpfan committed the suggested changes. thanks :)

thomasjpfan

I have already approved this. This needs a second approval to get merged.

fujiaxiang · 2020-06-03T10:43:55Z

got it. will wait for @NicolasHug to review again

fujiaxiang · 2020-06-05T10:10:32Z

ping

…w_2darray_with_1_feature

fujiaxiang · 2020-06-10T05:16:54Z

ping

fujiaxiang · 2020-06-15T15:29:08Z

hi @NicolasHug could you review again?

NicolasHug

thanks for your patience @fujiaxiang

doc/whats_new/v0.24.rst

sklearn/isotonic.py

…1_feature' into isotonic_reg_allow_2darray_with_1_feature

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

…w_2darray_with_1_feature

fujiaxiang · 2020-06-16T06:35:47Z

@NicolasHug thanks for the comments and suggestions. Have made the adjustments. Can you review again?

fujiaxiang · 2020-06-24T02:20:02Z

hi @NicolasHug can you review again?

jnothman · 2020-06-24T11:07:12Z

sklearn/tests/test_isotonic.py

+
+    y_pred1 = iso_reg.predict(X)
+    y_pred2 = iso_reg_2d.predict(X_2d)
+    assert_allclose(y_pred1, y_pred2)


Please also check that the estimator attributes match in shape. Thanks

Hi @jnothman, thanks for review. Are you referring to the _necessary_X_ and _necessary_y_ attributes? I have added checking for these two attributes. If it's something else, can you give a bit more details what you mean?

Sorry, I meant the public attributes X_min_, X_max_, etc.

hi @jnothman, sorry for the late reply. I have added the checks as requested. Could you review again?

…w_2darray_with_1_feature

…w_2darray_with_1_feature # Conflicts: # doc/whats_new/v0.24.rst # sklearn/tests/test_isotonic.py

jnothman

Thank you @fujiaxiang

…n#17379)

fujiaxiang added 2 commits May 29, 2020 11:37

allow isotonic reg 2d input with 1 feature (GH15012)

2a6ae7f

added PR number in whatsnew entry

b832a11

fujiaxiang added 2 commits May 29, 2020 11:58

corrected tag name in whatsnew entry

379dd23

added test to expect error when input has more than 1 feature

90c4da5

fujiaxiang changed the title ~~[WIP] Allow isotonic reg 2d input with 1 feature (GH15012)~~ [MRG] Allow isotonic reg 2d input with 1 feature (GH15012) May 29, 2020

thomasjpfan reviewed Jun 1, 2020

View reviewed changes

NicolasHug reviewed Jun 1, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

sklearn/isotonic.py Outdated Show resolved Hide resolved

sklearn/tests/test_isotonic.py Outdated Show resolved Hide resolved

sklearn/tests/test_isotonic.py Outdated Show resolved Hide resolved

fujiaxiang and others added 8 commits June 2, 2020 10:49

Update sklearn/isotonic.py

0424c32

change flatten to reshape(-1) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/isotonic.py

b3f1dfc

change flatten to reshape(-1) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

05eab7e

change array(list(range(10))) to arange(10) Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

f562f55

simplify test input Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

b507b1c

simplify test input Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

c9b8fd9

simplify code Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

1604cc6

change assert_array_equal to assert_allclose Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

refactored code

1b8f71b

fujiaxiang requested review from NicolasHug and thomasjpfan June 2, 2020 08:27

thomasjpfan approved these changes Jun 2, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

sklearn/tests/test_isotonic.py Outdated Show resolved Hide resolved

fujiaxiang and others added 2 commits June 2, 2020 22:57

Update doc/whats_new/v0.24.rst

75728d4

reformat whatsnew entry Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

Update sklearn/tests/test_isotonic.py

8d45ec6

clean up comment in test case Co-authored-by: Thomas J. Fan <thomasjpfan@gmail.com>

fujiaxiang requested a review from thomasjpfan June 2, 2020 15:51

thomasjpfan approved these changes Jun 2, 2020

View reviewed changes

fujiaxiang added 2 commits June 9, 2020 22:56

Merge branch 'master' into isotonic_reg_allow_2darray_with_1_feature

99aa688

Merge remote-tracking branch 'upstream/master' into isotonic_reg_allo…

17323ef

…w_2darray_with_1_feature

Merge branch 'master' into isotonic_reg_allow_2darray_with_1_feature

1f5290f

NicolasHug reviewed Jun 15, 2020

View reviewed changes

doc/whats_new/v0.24.rst Outdated Show resolved Hide resolved

sklearn/isotonic.py Outdated Show resolved Hide resolved

sklearn/isotonic.py Outdated Show resolved Hide resolved

sklearn/isotonic.py Outdated Show resolved Hide resolved

sklearn/isotonic.py Outdated Show resolved Hide resolved

fujiaxiang and others added 5 commits June 16, 2020 11:27

simplied whatsnew entry

c8e3670

Merge remote-tracking branch 'origin/isotonic_reg_allow_2darray_with_…

066fd9d

…1_feature' into isotonic_reg_allow_2darray_with_1_feature

Apply suggestions from code review

10e4fd2

Co-authored-by: Nicolas Hug <contact@nicolas-hug.com>

Merge remote-tracking branch 'upstream/master' into isotonic_reg_allo…

d5d27da

…w_2darray_with_1_feature

minor correction

ee3e4f5

fujiaxiang requested a review from NicolasHug June 16, 2020 06:34

jnothman reviewed Jun 24, 2020

View reviewed changes

fujiaxiang added 2 commits June 25, 2020 14:29

Merge remote-tracking branch 'upstream/master' into isotonic_reg_allo…

98f8db2

…w_2darray_with_1_feature

added attributes checking in unit test

0a5f65e

fujiaxiang requested a review from jnothman June 25, 2020 08:32

fujiaxiang added 2 commits July 6, 2020 11:05

Merge remote-tracking branch 'upstream/master' into isotonic_reg_allo…

102e587

…w_2darray_with_1_feature # Conflicts: # doc/whats_new/v0.24.rst # sklearn/tests/test_isotonic.py

added tests for estimator attributes

ebebdf5

jnothman approved these changes Jul 6, 2020

View reviewed changes

jnothman changed the title ~~[MRG] Allow isotonic reg 2d input with 1 feature (GH15012)~~ ENH Allow isotonic reg 2d input with 1 feature (GH15012) Jul 6, 2020

jnothman merged commit 4e6a6e2 into scikit-learn:master Jul 6, 2020

fujiaxiang deleted the isotonic_reg_allow_2darray_with_1_feature branch July 7, 2020 01:59

glemaitre pushed a commit to glemaitre/scikit-learn that referenced this pull request Jul 17, 2020

ENH Allow isotonic reg 2d input with 1 feature (GH15012) (scikit-lear…

7eb2864

…n#17379)

jayzed82 pushed a commit to jayzed82/scikit-learn that referenced this pull request Oct 22, 2020

ENH Allow isotonic reg 2d input with 1 feature (GH15012) (scikit-lear…

1b02433

…n#17379)

ENH Allow isotonic reg 2d input with 1 feature (GH15012) #17379

ENH Allow isotonic reg 2d input with 1 feature (GH15012) #17379

fujiaxiang commented May 29, 2020

fujiaxiang commented May 29, 2020

fujiaxiang commented May 29, 2020

thomasjpfan left a comment

NicolasHug left a comment

fujiaxiang commented Jun 2, 2020

thomasjpfan left a comment

fujiaxiang commented Jun 2, 2020

thomasjpfan left a comment

fujiaxiang commented Jun 3, 2020

fujiaxiang commented Jun 5, 2020

fujiaxiang commented Jun 10, 2020

fujiaxiang commented Jun 15, 2020

NicolasHug left a comment

fujiaxiang commented Jun 16, 2020

fujiaxiang commented Jun 24, 2020

jnothman Jun 24, 2020

fujiaxiang Jun 25, 2020

jnothman Jun 25, 2020

fujiaxiang Jul 6, 2020

jnothman left a comment

ENH Allow isotonic reg 2d input with 1 feature (GH15012) #17379

ENH Allow isotonic reg 2d input with 1 feature (GH15012) #17379

Conversation

fujiaxiang commented May 29, 2020

Reference Issues/PRs

What does this implement/fix? Explain your changes.

fujiaxiang commented May 29, 2020

fujiaxiang commented May 29, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

NicolasHug left a comment

Choose a reason for hiding this comment

fujiaxiang commented Jun 2, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

fujiaxiang commented Jun 2, 2020

thomasjpfan left a comment

Choose a reason for hiding this comment

fujiaxiang commented Jun 3, 2020

fujiaxiang commented Jun 5, 2020

fujiaxiang commented Jun 10, 2020

fujiaxiang commented Jun 15, 2020

NicolasHug left a comment

Choose a reason for hiding this comment

fujiaxiang commented Jun 16, 2020

fujiaxiang commented Jun 24, 2020

jnothman Jun 24, 2020

Choose a reason for hiding this comment

fujiaxiang Jun 25, 2020

Choose a reason for hiding this comment

jnothman Jun 25, 2020

Choose a reason for hiding this comment

fujiaxiang Jul 6, 2020

Choose a reason for hiding this comment

jnothman left a comment

Choose a reason for hiding this comment