MNT Import linear assignment from scipy by praths007 · Pull Request #13465 · scikit-learn/scikit-learn

praths007 · 2019-03-18T11:28:09Z

Reference Issues/PRs

linear_assignment can now be imported from scipy #13464

What does this implement/fix? Explain your changes.

Removed the use of sklearn.utils.linear_assignment_ and _hungarian and replaced it with scipy.optimize.linear_sum_assignment

Tweaked test case for _hungarian so that it works with scipy.optimize.linear_sum_assignment

Any other comments?

more info can be found here #13464

merge fork

jnothman

Thanks!

jnothman · 2019-03-18T11:54:09Z

sklearn/utils/linear_assignment_.py

+as the algorithm incorporated in SciPy will be used instead.
+
 """
 # Based on original code by Brain Clapper, adapted to NumPy by Gael Varoquaux.


please use the warnings module to raise a deprecation warning here (so that it appears whenever the module is imported)

jnothman · 2019-03-18T11:54:47Z

sklearn/utils/tests/test_linear_assignment.py

@@ -4,7 +4,7 @@
 import numpy as np


we don't need this file. It can be removed entirely. Scipy tests their implementation.

Actually let's leave it in to maintain code coverage. Use the version in master, only changing it to ignore deprecation warnings during import. That way we will be sure to remove it in 0.23.

…gnore

jnothman · 2019-03-19T01:56:15Z

sklearn/utils/linear_assignment_.py

+# Deprecation warning for module
+warnings.warn(
+   "This module is deprecated in 0.21 and will removed from 0.23 "
+   "as the algorithm incorporated in SciPy will be used instead.", ImportWarning)


Use a DeprecationWarning, rather

jnothman

Sorry, that was in response to an old change.

jnothman · 2019-03-19T01:57:19Z

sklearn/utils/tests/test_linear_assignment.py

 import numpy as np

 # XXX we should be testing the public API here
 from sklearn.utils.linear_assignment_ import _hungarian


you need to ignore the warning here too.. or just put the import inside the test function

moved the import inside test function

jnothman · 2019-03-19T08:40:30Z

sklearn/utils/tests/test_linear_assignment.py

+
 import numpy as np

 # XXX we should be testing the public API here


Remove this and the spare blank lines

NicolasHug

Some comments

NicolasHug · 2019-03-22T18:12:50Z

sklearn/utils/tests/test_linear_assignment.py

 # License: BSD

-import numpy as np
+# TODO: Remove this test module as the methods being tested


Please add a line above with # 0.23 so we know when to remove this file

NicolasHug · 2019-03-22T18:14:36Z

sklearn/utils/linear_assignment_.py

        the original ordering.
    """
+    warnings.warn(
+        "This method is deprecated in 0.21 and will removed from 0.23 "


Maybe indicate the function name and what to use instead:

The linear_assignment function is deprecated [...] Use scipy.optimize.linear_sum_assignment instead

NicolasHug · 2019-03-22T18:15:21Z

sklearn/utils/linear_assignment_.py

+
+# Deprecation warning for module
+warnings.warn(
+    "This module is deprecated in 0.21 and will removed from 0.23 "


Suggested change

"This module is deprecated in 0.21 and will removed from 0.23 "

"The linear_assignment_ module is deprecated in 0.21 and will removed from 0.23 "

NicolasHug · 2019-03-22T18:16:18Z

sklearn/metrics/cluster/bicluster.py

        similarity = _jaccard
    matrix = _pairwise_similarity(a, b, similarity)
-    indices = linear_assignment(1. - matrix)
+    row, col = linear_sum_assignment(1. - matrix)


Suggested change

row, col = linear_sum_assignment(1. - matrix)

row_indices, col_indices = linear_sum_assignment(1. - matrix)

NicolasHug

Some comments

NicolasHug · 2019-03-25T11:39:53Z

sklearn/metrics/cluster/bicluster.py


-from sklearn.utils.linear_assignment_ import linear_assignment
+from scipy.optimize import linear_sum_assignment
 from sklearn.utils.validation import check_consistent_length, check_array


It's not you, but can you please change this toa relative import:

from ...utils.validation import check_consistent_length, check_array

Also Nitpick for pep8 compliance: the scipy import should be right below the numpy import, and there should be a blank line before the sklearn import:

import numpy as np from scipy.optimize import linear_sum_assignment from ...utils.validation import check_consistent_length, check_array

NicolasHug · 2019-03-25T11:40:43Z

sklearn/utils/linear_assignment_.py

        the original ordering.
    """
+    warnings.warn(
+        "The _hungarian function is deprecated in 0.21 "


Suggested change

"The _hungarian function is deprecated in 0.21 "

"The linear_assignment function is deprecated in 0.21 "

NicolasHug

Last comment, LGTM, thanks @praths007

NicolasHug · 2019-03-25T12:10:35Z

sklearn/utils/linear_assignment_.py

+
+# Deprecation warning for module
+warnings.warn(
+    "The linear_assignment function is deprecated in 0.21 "


Suggested change

"The linear_assignment function is deprecated in 0.21 "

"The linear_assignment_ module is deprecated in 0.21 "

jnothman · 2019-04-17T10:00:34Z

Thanks @praths007

This reverts commit a57cd4e.

praths007 added 5 commits March 4, 2019 07:36

Merge pull request #1 from scikit-learn/master

ba5fd14

merge fork

Merge pull request #2 from scikit-learn/master

972d9e6

merge fork

Merge pull request #3 from scikit-learn/master

80074cb

merge fork

importing linear assignment from scipy

1f78d98

adding deprecation to docstring, removed TODO from module

6a49939

jnothman reviewed Mar 18, 2019

View reviewed changes

praths007 added 4 commits March 18, 2019 08:04

removed test_linear assignment, added ImportWarning

008ee5d

flake 8 adjustment

1476371

reverted deletion of test_linear_assignment file, added deprecation i…

dd13ff2

…gnore

added TODO to remove test_linear_assignment

7e52f7c

jnothman reviewed Mar 19, 2019

View reviewed changes

move import inside test function

a6a1189

jnothman reviewed Mar 19, 2019

View reviewed changes

removed comment and blank lines

d33177f

jnothman approved these changes Mar 19, 2019

View reviewed changes

jnothman changed the title ~~Import linear assignment from scipy~~ MNT Import linear assignment from scipy Mar 19, 2019

This was referenced Mar 19, 2019

linear_assignment can now be imported from scipy #13464

Closed

Add return_predictions option to the model_selection.cross_validate() API #13478

Open

NicolasHug reviewed Mar 22, 2019

View reviewed changes

praths007 added 3 commits March 25, 2019 07:15

incoporated review changes #1

2f53b13

changed deprecation warning

98fa714

change v to #

a70841e

NicolasHug reviewed Mar 25, 2019

View reviewed changes

review changes #2

4ab9aa2

NicolasHug approved these changes Mar 25, 2019

View reviewed changes

review changes #3

e2d364f

jnothman merged commit cfaf352 into scikit-learn:master Apr 17, 2019

jeremiedbb pushed a commit to jeremiedbb/scikit-learn that referenced this pull request Apr 25, 2019

MNT Import linear assignment from scipy (scikit-learn#13465)

f11f647

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

MNT Import linear assignment from scipy (scikit-learn#13465)

a57cd4e

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "MNT Import linear assignment from scipy (scikit-learn#13465)"

52f0a81

This reverts commit a57cd4e.

xhluca pushed a commit to xhluca/scikit-learn that referenced this pull request Apr 28, 2019

Revert "MNT Import linear assignment from scipy (scikit-learn#13465)"

db0a833

This reverts commit a57cd4e.

koenvandevelde pushed a commit to koenvandevelde/scikit-learn that referenced this pull request Jul 12, 2019

MNT Import linear assignment from scipy (scikit-learn#13465)

debcdcd

heyuanfly mentioned this pull request Jun 10, 2020

ModuleNotFoundError: No module named 'sklearn.utils.linear_assignment_' ahmetozlu/tensorflow_object_counting_api#78

Closed


		import numpy as np

		# XXX we should be testing the public API here

	"This module is deprecated in 0.21 and will removed from 0.23 "
	"The linear_assignment_ module is deprecated in 0.21 and will removed from 0.23 "

	row, col = linear_sum_assignment(1. - matrix)
	row_indices, col_indices = linear_sum_assignment(1. - matrix)

	"The _hungarian function is deprecated in 0.21 "
	"The linear_assignment function is deprecated in 0.21 "

Uh oh!

Conversation

praths007 commented Mar 18, 2019 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Reference Issues/PRs

What does this implement/fix? Explain your changes.

Any other comments?

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

NicolasHug left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jnothman commented Apr 17, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

praths007 commented Mar 18, 2019 •

edited

Loading