-
-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Trivial image check for peak_local_max #3990
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.
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
Comments
If we choose to remove this trivial check, I think it would be worth adding it back to corner_peaks, as I think this function can easily face trivial images if corner detectors fail. |
I see one major drawback to the current implementation of the trivial image check: when >>> from skimage.feature import peak_local_max
>>> img = np.random.rand(10, 10)
>>> img[2:-2, 2:-2] = 1
>>> labels = np.zeros((10, 10), int)
>>> labels[2:-2, 2:-2] = 1
>>> out = peak_local_max(img, labels=labels, indices=False) We can, like @hmaarrfk is proposing, remove the test, An other major drawback I see in the current implementation, is the use of the |
As proposed in #4751, using a generator to perform the test makes it O(1) instead of O(n). |
Description
peak_local_max
, current tests for trivial images. That is, if an image is constant, and exactly constant, then it will return that no local peaks are found. If even one pixel is off, then it will return that many local peaks were found.I think this test is too much of an edge case, and should simply be removed possibly without deprecation.
The relevant code is:
(not linking to the original code since it will be obsolete soon).
I think this is such an edge case that it isn't worth the cost imposed on checking equality on every other user of this function.
The users that may have trivial images in their analysis pipelines should find better ways of detecting them.
We've tried to think of better ways for testing for the trivial case, but due to the amount of options that the peak_local_max function provides, it seems like it is more complicated than it seems at first glance:
#3984 (comment)
cc: @clementkng and @brownmk
xref: #3984
The text was updated successfully, but these errors were encountered: