I wouldn't be concerned about allocating a mask of bools the same size as the input array. Most of the operations on the image data are going to create temporary arrays of floats, eight times bigger. You could conserve some memory by reusing the variable: mask = binary_erosion(mask, ...) but that's sacrificing a bit of clarity for probably a not very large savings. Speaking of clarity, I would replace "emask" with "eroded_mask". Given the arguments above, I suggest moving mask to the last argument, make it a keyword with a default of None, which is replaced with ones(image.shape, dtype=bool) Also, I would move the smooth_with_function_and_mask() into canny.py and remove smooth.py, for now. The other functions in smooth.py may be needed in the future, but not yet. Thouis Jones On Thu, Mar 31, 2011 at 01:02, Dan Farmer <dfarmernv@gmail.com> wrote:
https://github.com/dfarmer/scikits.image/compare/master...dfarmer-filters-ca...
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