[SciPy-Dev] ndimage grey morphology tickets

[Tony Yu]

On Mon, May 7, 2012 at 3:38 PM, Jonathan Helmus <jjhelmus at gmail.com> wrote:

> All,
>
> Trac tickets #1135, #1281 and #1498 point out bugs in the
> ndimage.grey_dilation and grey_erosion function.  I've started a branch
> that tries to address these issues:
> https://github.com/jjhelmus/scipy/tree/grey_morphology-fix
>
> This branch currently passes the tests in test_ndimage.py and the
> dilation_test.py file attached to ticket 1135, but I am not certain on
> two issues that I was hoping someone here might be able to comment on them:
>
> 1.  Should there be a sanity check on the shape of footprint, structure
> and size when more than one is provided?
> 2.  Ticket #1281 points out that grey_erosion does not have the
> parameter checking that grey_dilation has.  I added these checks but
> noticed that  to pass tests structure and footprint should not be
> reversed and the origin negated.  Is this correct?  If so the commented
> out lines in the branch should will be deleted.
>
> If this would be better discussed in a pull request I'd be happy to make
> one.
>
> http://projects.scipy.org/scipy/ticket/1135
> http://projects.scipy.org/scipy/ticket/1281
> http://projects.scipy.org/scipy/ticket/1498
>
>
> Cheers,
>
>    -Jonathan Helmus
>
> Hi Jonathan,

I recently submitted a fix for ticket
1135<https://github.com/scipy/scipy/pull/199/files>,
but I realize now that I should have pinged the list. I didn't actually
know about tickets 1281 or 1498. I think the PR I submitted should also
take care of 1498, but I don't think 1281 is actually a bug.

IIRC, the reason for the difference is that dilate shifts the origin of the
structuring element/footprint if its size is even-numbered (i.e. doesn't
have a "center" pixel). This shift makes dilation and erosion
reversible---otherwise applying one, then the other would shift
features. Are you sure the changes to `grey_erosion` are necessary?

Also, I trimmed down the tests from PR 1135 (which seem to be the same
tests in your branch) so that they only test for the submitted change. I
think I originally wrote the tests (it's been more than 2 years so I could
be wrong about that), and I was just testing everything related to the
change. I trimmed it down because some tests replicated existing tests (and
`test_ndimage.py` is already really long).

I haven't had a chance to carefully look over your branch to compare, but I
will tonight.

-Tony
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.python.org/pipermail/scipy-dev/attachments/20120507/f930f9b5/attachment.html>