Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

skimage test broken by minmax filter change #3898

Closed
juliantaylor opened this issue Aug 24, 2014 · 4 comments
Closed

skimage test broken by minmax filter change #3898

juliantaylor opened this issue Aug 24, 2014 · 4 comments
Labels
scipy.ndimage
Milestone
0.15.0

Comments

@juliantaylor
Copy link
Contributor

PR #3517 (commit 11bd10e) breaks a skimage test:

python /usr/lib/python2.7/dist-packages/skimage/transform/tests/test_hough_transform.py
......F...........
======================================================================
FAIL: test_hough_transform.test_hough_line_peaks_num
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/usr/lib/python2.7/dist-packages/nose/case.py", line 197, in runTest
    self.test(*self.arg)
  File "/usr/lib/python2.7/dist-packages/skimage/transform/tests/test_hough_transform.py", line 120, in test_hough_line_peaks_num
    min_angle=0, num_peaks=1)[0]) == 1
AssertionError

----------------------------------------------------------------------
Ran 18 tests in 0.786s

@jaimefrio
@juliantaylor juliantaylor mentioned this issue Aug 24, 2014
Closed
@rgommers rgommers added this to the 0.15.0 milestone Aug 24, 2014
@jaimefrio
Copy link
Member

That test is calling minimum_filter_1d a couple of times with a filter size of 1, and this much simpler test replicates the wrong behavior:

>>> import numpy as np
>>> from scipy.ndimage import minimum_filter1d
>>> minimum_filter1d(np.arange(10), 1)
array([1, 2, 3, 4, 5, 6, 7, 8, 9, 0])

The code is indeed broken for this case, but it shouldn't be hard to fix. I may take this as an opportunity to look into getting #3527 going, which would be a better all around solution, but is also broken like this one.

And I guess the tests also need some improving.

@otizonaizit
Copy link
Contributor

I am working on this one.

@otizonaizit otizonaizit mentioned this issue Aug 31, 2014
Closed
jaimefrio added a commit to jaimefrio/scipy that referenced this issue Sep 1, 2014
@jaimefrio
ENH: min(max)imum_filter1d using the MINLIST algorithm, fixes scipy#3898
5b8f87a 


This PR implements the minimum_filter1d and maximum_filter1d functions
using Richard Harter's implementation of the MINLIST algorithm. While
slower (~30%) than the MINLINE algorithm currently in place for random
inputs, its performance is consistent for all inputs, and is much
faster for long, ordered seqeunces.
@jaimefrio jaimefrio mentioned this issue Sep 1, 2014
Closed
jaimefrio added a commit to jaimefrio/scipy that referenced this issue Sep 1, 2014
@jaimefrio
ENH: min(max)imum_filter1d using the MINLIST algorithm, fixes scipy#3898
598c922 


This PR implements the minimum_filter1d and maximum_filter1d functions
using Richard Harter's implementation of the MINLIST algorithm. While
slower (~30%) than the MINLINE algorithm currently in place for random
inputs, its performance is consistent for all inputs, and is much
faster for long, ordered seqeunces.
@jaimefrio
Copy link
Member

@juliantaylor I tried building skimage myself, but am getting several errors and a ton of warnings when I run their tests. The one you found is not one of them, but could you, just to be sure, rerun them with the current #3527?

@juliantaylor
Copy link
Contributor Author

@jaimefrio your PR fixes the skimage issue

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
scipy.ndimage
Projects
None yet
Milestone
0.15.0
Development

Successfully merging a pull request may close this issue.

Regression test and fix for #3898
4 participants
@rgommers @otizonaizit @juliantaylor @jaimefrio