Added a check for empty array in _shared.utils.py #2364

tseclaudia · 2016-11-01T08:12:25Z

Description

Addresses Issue #2053

soupault · 2016-11-01T08:21:51Z

@tseclaudia Hi! Thank for catching this up!
In order to close #2053 you'll need to address the 2nd part of the issue as well: adding assert_nD to feature.local_binary_pattern. Should also add a simple test which fails on a current master but will be fixed with your patch merged.

codecov-io · 2016-11-01T09:05:01Z

Current coverage is 90.60% (diff: 27.27%)

Merging #2364 into master will decrease coverage by 0.04%

@@             master      #2364   diff @@
==========================================
  Files           305        305          
  Lines         21582      21605    +23   
  Methods           0          0          
  Messages          0          0          
  Branches       1853       1857     +4   
==========================================
+ Hits          19563      19575    +12   
- Misses         1665       1674     +9   
- Partials        354        356     +2

Powered by Codecov. Last update 3551389...f642de0

tseclaudia · 2016-11-01T19:57:22Z

@soupault Doesn't feature.local_binary_pattern already contain assert_nD which checks that the passed in array meets the desired ndims and isn't an empty array? Does that correctly address the second part of the issue?

… array is passed into features.local_binary_pattern

soupault · 2016-11-01T20:32:50Z

@tseclaudia fair enough, sorry. OK, then I'd like to ask you to add a test to _shared/tests/test_utils.py checking if assert_nD raises an error on empty array.
Oh, and, please, follow code style conventions (i.e. wrap at 80th column).

soupault · 2016-11-02T08:59:32Z

skimage/_shared/tests/test_utils.py

+    try:
+        z = np.random.random(200**2).reshape((200, 200))
+        x = z[10:30, 30:10]
+        assert_nD(x, 2)


There is no need to use try/except here. Just use numpy.testing.assert_raises, i.e.

from numpy.testing import assert_raises ... assert_raises(ValueError, assert_nD, x, 2)

soupault · 2016-11-02T08:59:51Z

skimage/feature/tests/test_texture.py

+        #     z = np.random.random(200**2).reshape((200, 200))
+        #     x = z[10:30, 30:10]
+        #     lbp = feature.local_binary_pattern(x, 8, 1, 'uniform')
+        try:


soupault · 2016-11-02T09:00:01Z

skimage/feature/tests/test_texture.py

@@ -228,6 +229,22 @@ def test_uniform(self):
                        [2, 9, 0, 8, 1, 2]])
        np.testing.assert_array_equal(lbp, ref)

+    def test_empty(self):
+        # with unittest.TestCase.assertRaises(ValueError):


Remove the comments.

soupault · 2016-11-02T09:01:29Z

skimage/_shared/utils.py

@@ -147,7 +147,7 @@ def safe_as_int(val, atol=1e-3):

 def assert_nD(array, ndim, arg_name='image'):
    """
-    Verify an array meets the desired ndims.
+    Verify an array meets the desired ndims and array isn't empty.


Did you accidentally remove the proposed fix?

…s.py

soupault · 2016-11-14T11:24:21Z

skimage/_shared/tests/test_utils.py

+
+
+def test_assert_nD():
+    try:


Remove try:

soupault · 2016-11-14T11:25:09Z

skimage/_shared/tests/test_utils.py

+    try:
+        z = np.random.random(200**2).reshape((200, 200))
+        x = z[10:30, 30:10]
+        assert_raises(ValueError, assert_nDx, 2)


-> assert_raises(ValueError, assert_nD, x, 2)

jni

It looks like the coverage has decreased because the new test is incorrect...?

jni · 2016-11-28T03:53:36Z

skimage/_shared/tests/test_utils.py

@@ -1,7 +1,16 @@
-from skimage._shared.utils import copy_func
+from skimage._shared.utils import (copy_func, 


one line: from skimage._shared.utils import copy_func, assert_nD

jni · 2016-11-28T03:54:19Z

skimage/_shared/tests/test_utils.py

+def test_assert_nD():
+    z = np.random.random(200**2).reshape((200, 200))
+    x = z[10:30, 30:10]
+    assert_raises(ValueError, assert_nD, 2)


Should this be assert_nD, x, 2?

Not yet ready

soupault · 2016-12-01T13:31:34Z

Thanks @tseclaudia!

added a check for empty array in _shared.utils.py

7271a98

soupault added bug 💪 Work in progress labels Nov 1, 2016

added test for checking that an error message is raised when an empty…

6b89422

… array is passed into features.local_binary_pattern

tseclaudia added 2 commits November 1, 2016 13:33

check for empty arrays in utils.assert_nD

87f3d87

added test for utils.assert_nD in test_utils.py

8287894

soupault requested changes Nov 2, 2016

View reviewed changes

replaced replaced try/except with assert_raises in _shared/tests/util…

bc84ab6

…s.py

soupault requested changes Nov 14, 2016

View reviewed changes

fixed test_assert_nD

f642de0

soupault previously approved these changes Nov 15, 2016

View reviewed changes

soupault added status: mrg+1 and removed 💪 Work in progress labels Nov 18, 2016

jni requested changes Nov 28, 2016

View reviewed changes

soupault removed the status: mrg+1 label Nov 28, 2016

fixed syntax of imports and test_assert_nD

d5c58d4

jni approved these changes Dec 1, 2016

View reviewed changes

soupault merged commit 25fef7f into scikit-image:master Dec 1, 2016

soupault mentioned this pull request Dec 1, 2016

feature.local_binary_pattern fails on empty array sliced from non-contiguous source #2053

Closed

3 tasks

tseclaudia deleted the check-for-empty-array-in-assert_nD branch December 1, 2016 20:47

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added a check for empty array in _shared.utils.py #2364

Added a check for empty array in _shared.utils.py #2364

tseclaudia commented Nov 1, 2016

soupault commented Nov 1, 2016 •

edited

codecov-io commented Nov 1, 2016 •

edited

tseclaudia commented Nov 1, 2016

soupault commented Nov 1, 2016

soupault Nov 2, 2016

soupault Nov 2, 2016

soupault Nov 2, 2016

soupault Nov 2, 2016

soupault Nov 14, 2016

soupault Nov 14, 2016

jni left a comment

jni Nov 28, 2016

jni Nov 28, 2016

soupault commented Dec 1, 2016

		@@ -1,7 +1,16 @@
		from skimage._shared.utils import copy_func
		from skimage._shared.utils import (copy_func,

Added a check for empty array in _shared.utils.py #2364

Added a check for empty array in _shared.utils.py #2364

Conversation

tseclaudia commented Nov 1, 2016

Description

soupault commented Nov 1, 2016 • edited

codecov-io commented Nov 1, 2016 • edited

Current coverage is 90.60% (diff: 27.27%)

tseclaudia commented Nov 1, 2016

soupault commented Nov 1, 2016

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jni left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

soupault commented Dec 1, 2016

soupault commented Nov 1, 2016 •

edited

codecov-io commented Nov 1, 2016 •

edited