Adding new functionality/cleaning up skimage wrappers to ndi.morphology functions

[nrweir]

Description

This PR updates skimage.morphology.binary and skimage.morphology.grey morphology functions to utilize the new arguments available in the ndi.morphology functions they wrap, as per #2597.

Previously, skimage.morphology.binary and skimage.morphology.grey did not always directly utilize the ndi.morphology function equivalent (e.g. for skimage.morphology.binary.binary_closing, the function sequentially called the skimage.morphology.binary.binary_dilation followed by skimage.morphology.binary.binary_erosion (which are wrappers to ndi.morphology functions by the same names) rather than directly calling ndi.morphology.binary_closing. For clarity and simplicity, these functions were converted to direct wrappers for the ndi functions.

Docstrings have been edited to point out that these skimage.morphology functions are wrappers to the equivalent ndi.morphology functions.

Because of slight differences between how the skimage.morphology functions handled edges compared to the default mirroring implemented by most ndi.morphology functions, this required small changes to a couple of the tests:

• skimage.morphology.grey.black_tophat tests had slightly different outputs as documented in the ENH: pass options from scipy.ndimage to our wrappers #2597 discussion. The expected test outputs were changed accordingly. Similarly,
• skimage.morphology.grey.opening produced slightly different edge behavior when made into a cleaner wrapper for ndi.morphology.grey_opening.

These two changes were implemented by updating skimage/tests/gray_morph_output.npz.

One additional change was necessary to handle testing of eccentrically shaped strels in skimage.morphology.tests.test_grey.TestEccentricStructuringElements. The reasoning and change is documented there.

Checklist

[It's fine to submit PRs which are a work in progress! But before they are merged, all PRs should provide:]

• Clean style in the spirit of PEP8
• Docstrings for all functions
• Gallery example in ./doc/examples (new features only)
• Unit tests

Unit tests already existed for all of these functions, but tests don't currently exist to test the additional functionality generated by implementing the new arguments available to ndi.morphology functions. This is my next order of business.

References

#2597

For reviewers

(Don't remove the checklist below.)

• Check that the PR title is short, concise, and will make sense 1 year
  later.
• Check that new functions are imported in corresponding __init__.py.
• Check that new features, API changes, and deprecations are mentioned in
  doc/release/release_dev.rst.

[pep8speaks]

Hello @nrweir! Thanks for updating the PR.

Cheers ! There are no PEP8 issues in this Pull Request. 🍻

Comment last updated on December 16, 2017 at 01:48 Hours UTC

[codecov-io]

Codecov Report

Merging #2917 into master will decrease coverage by 0.02%.
The diff coverage is 89.61%.

@@            Coverage Diff             @@
##           master    #2917      +/-   ##
==========================================
- Coverage   86.04%   86.01%   -0.03%     
==========================================
  Files         337      337              
  Lines       27032    27052      +20     
==========================================
+ Hits        23259    23270      +11     
- Misses       3773     3782       +9

Impacted Files	Coverage Δ
skimage/morphology/tests/test_binary.py	99.25% <100%> (+0.12%)	⬆️
skimage/morphology/binary.py	100% <100%> (ø)	⬆️
skimage/morphology/tests/test_grey.py	100% <100%> (ø)	⬆️
skimage/morphology/grey.py	86.04% <69.23%> (-9.79%)	⬇️
skimage/draw/_random_shapes.py	95.06% <0%> (-1.24%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update e2a6094...c9f4300. Read the comment docs.

[nrweir]

@soupault @stefanv @emmanuelle @jni looks like this passes the various build tests, let me know if you need more on my end.

[stefanv]

@nrweir Any reason why you closed the PR?

[nrweir]

@stefanv sorry, I wanted to clean up the messy commit history and add the new tests for the new arguments, and was going to generate a bunch of confusing commits in the process. I'll reopen shortly once I fix everything up. That ok?

[stefanv]

OK, that's perfectly fine! In the future, you can also rename the PR with the "WIP: ..." prefix, and then keep pushing. We squash commits in the end anyway now, so having many is no problem. Thanks for working on this, Nicholas!

[nrweir]

Got it, thanks, and sorry about the confusion! I already had a pretty ugly commit history and wanted to clean it up so it was easier for reviewers to follow. It'll be re-opened shortly.

…

On Thu, Dec 14, 2017 at 2:07 AM, Stefan van der Walt < ***@***.***> wrote: OK, that's perfectly fine! In the future, you can also rename the PR with the "WIP: ..." prefix, and then keep pushing. We squash commits in the end anyway now, so having many is no problem. Thanks for working on this, Nicholas! — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#2917 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AMa2oyk7e2yTU5UcQTVz_mQZ_wGb6Ztpks5tAMkigaJpZM4Q8zoH> .

-- Nicholas Weir Postdoctoral Fellow, Denic Laboratory Dept. of Molecular and Cellular Biology Harvard University <http://www.linkedin.com/in/niweir/> <http://www.github.com/nrweir/> <http://www.deniclab.com/>

[nrweir]

I'm having some issues with Travis builds where there's no obvious issue with my code (passes tests/etc.), but for some reason the build fails during flake8 testing. Couple of examples:

https://travis-ci.org/nrweir/scikit-image/jobs/317029332 where it throws an IOError
https://travis-ci.org/nrweir/scikit-image/jobs/317029334 where there's no error at all, it just cuts off in the middle of the flake8 test

Can someone help me figure out what's wrong? Thank you!!

[nrweir]

(Seems like it only has these flake8 issues in my own Travis-CI, not in the scikit-image build test, so whatever...)