Review: Canny

[Stéfan van der Walt]

Hi Dan

On Thu, Mar 31, 2011 at 1:02 AM, Dan Farmer <dfarmernv at gmail.com> wrote:
> https://github.com/dfarmer/scikits.image/compare/master...dfarmer-filters-canny

I read through your patch and made some preliminary comments.

> Mostly just trying to follow procedure. I already mentioned my
> concerns in the previous thread. I made one stab at introduced a
> "None" default for the mask, but I got hung up and reverted it. The
> default I was going to propose was np.ones(img.shape,bool) (and after
> the fact I even noticed that's how it is used in one of the unit
> tests). But I started thinking that that could be quite wasteful of
> memory if you were working with large images (on my test use case with
> ~512x512 images it's about 300 KB for the "fake" mask).

It seems as though this specific implementation of the algorithms
relies on creating the mask, so I don't think you can get away from
it.  The typical way to do it would be:

def canny(..., mask=None, ...):
    if mask is None:
        mask = np.ones(x.shape, dtype=bool)

> The problem I had was that if I don't allocate the emask array I get
> run-time errors starting at line 129 (in the diff of canny.py) because
> the arrays all have different lengths if they aren't logical_and'd
> with emask above.

Yes, I think the only way to avoid allocating the mask explicitly is
to rewrite the algorithm in Cython, where you can modify behaviour
inside the for-loop.

Regards
Stéfan