Review: Canny

Dan Farmer dfarmernv at gmail.com
Thu Mar 31 13:26:25 EDT 2011


I will try to address your comments tonight and also see about
removing smooth.py and just incorporating the one function we're using
into canny.py for now as Thouis suggested. Thanks to both of you for
looking at it. I'll message back when I've pushed the changes.

-Dan

2011/3/31 Stéfan van der Walt <stefan at sun.ac.za>:
> 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
>



More information about the scikit-image mailing list