Review: Canny

Dan Farmer dfarmernv at gmail.com
Wed Mar 30 19:02:40 EDT 2011


https://github.com/dfarmer/scikits.image/compare/master...dfarmer-filters-canny

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).

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.

So anyway, I thought I would let you take a look at the code and
decide if you'd like me to try to figure out a way around that or
incorporate it as is (with no default) or add the np.ones default.

The tests pass and as I mentioned it has worked fine so far in
day-to-day use. Also regarded Ralf's comment to consolidate into one
filter.py file, I agree with that I just figured I would do that with
the next batch (in a separate commit). But again, let me know if I'm
not approaching this properly.

Thanks,
Dan



More information about the scikit-image mailing list