Almost ready to issue pull for Canny?

Hi Group, I merged Cell Profiler's Canny over last weekend and spent this week "dogfooding" it on my own project, I had a few questions and ideas to run by before I push it back to github. 1. Organization: In general the CP filter code is mostly in one file, I tried to follow the example of the constant time median filter merge that was already done. I just wanted to make sure that you guys want one filter per file? 2. API: So I used the function in part of a project I've been doing this semester and my call to canny looks like this: canny(img, np.ones(img.shape, dtype=bool), 5, 0.005, 0.01), the np.ones bit is because the Cell Profiler canny (as far as I can tell) requires a binary mask, and the 0.005 and 0.01 are the low and high thresholds for hysteresis thresholding. As you can image that took me quit a bit of fiddling to get those values. It seems that since it's thresholding the magnitude values you wind up with strange values. So my thoughts were: can we add a default parameter for the mask so that end-users of the library don't have to think about it unless they want to (I do think it's a nice functionality to have), and secondly should anything be done (like normalizing between 0 and 1) about the pixel values so that there can be some intuition about threshold values? 3. As far as the pull request goes, do I just merge my local canny branch back into master locally and then push that to github or do I actually push the branch to github and issue a pull against that? Thanks, Dan

I would definitely add a default for the mask, as a keyword argument with None indicating a full-on mask. I'm not very familiar with the code, but it seems like making the thresholds be relative to the image.min() and image.max() would be fine, or it might be more robust having them set relative to the median pixel value and MAD, or 5th and 95th percentiles. It looks like matlab chooses them relative to the maximum gradient magnitude. Lee - any opinions? Ray Jones On Sat, Mar 26, 2011 at 15:41, Dan Farmer <dfarmernv@gmail.com> wrote:
Hi Group,
I merged Cell Profiler's Canny over last weekend and spent this week "dogfooding" it on my own project, I had a few questions and ideas to run by before I push it back to github.
1. Organization: In general the CP filter code is mostly in one file, I tried to follow the example of the constant time median filter merge that was already done. I just wanted to make sure that you guys want one filter per file? 2. API: So I used the function in part of a project I've been doing this semester and my call to canny looks like this: canny(img, np.ones(img.shape, dtype=bool), 5, 0.005, 0.01), the np.ones bit is because the Cell Profiler canny (as far as I can tell) requires a binary mask, and the 0.005 and 0.01 are the low and high thresholds for hysteresis thresholding. As you can image that took me quit a bit of fiddling to get those values. It seems that since it's thresholding the magnitude values you wind up with strange values. So my thoughts were: can we add a default parameter for the mask so that end-users of the library don't have to think about it unless they want to (I do think it's a nice functionality to have), and secondly should anything be done (like normalizing between 0 and 1) about the pixel values so that there can be some intuition about threshold values? 3. As far as the pull request goes, do I just merge my local canny branch back into master locally and then push that to github or do I actually push the branch to github and issue a pull against that?
Thanks, Dan

On Sat, Mar 26, 2011 at 3:41 PM, Dan Farmer <dfarmernv@gmail.com> wrote:
Hi Group,
I merged Cell Profiler's Canny over last weekend and spent this week "dogfooding" it on my own project, I had a few questions and ideas to run by before I push it back to github.
1. Organization: In general the CP filter code is mostly in one file, I tried to follow the example of the constant time median filter merge that was already done. I just wanted to make sure that you guys want one filter per file?
I find the files under filter/ too short now, that could all be in a single file IMHO. Python is not Matlab:)
3. As far as the pull request goes, do I just merge my local canny branch back into master locally and then push that to github or do I actually push the branch to github and issue a pull against that?
It's better not to do any merges into or from master. Just push your branch to github and then send a pull request. There's documentation on how to work with git branches at http://stefanv.github.com/scikits.image/gitwash/development_workflow.html Cheers, Ralf

Hi Dan On Sat, Mar 26, 2011 at 4:41 PM, Dan Farmer <dfarmernv@gmail.com> wrote:
I merged Cell Profiler's Canny over last weekend and spent this week "dogfooding" it on my own project, I had a few questions and ideas to run by before I push it back to github.
Fantastic! It looks like Ralf and Thouis answered most of your questions. Need me know if you need anything else before making a pull request. You may also have a look at http://stefanv.github.com/scikits.image/contribute.html#development-process for a description of our development process. Thanks! Stéfan
participants (4)
-
Dan Farmer
-
Ralf Gommers
-
Stéfan van der Walt
-
Thouis (Ray) Jones