On Mon, Jan 9, 2012 at 4:36 AM, Nelle Varoquaux <nelle.varoquaux@gmail.com>wrote:

I think it is a good idea to do a such a method: maybe in the util module ? On the other hand, the harris response by itself is totally useless, so I suggest to keep it in "one chunk".

N

Actually, I don't think this is much of a problem: if the peak detection part was factored out (maybe keeping it in the `feature` module instead of moving it to `util`), then you could just have the `harris` function call `_compute_harris_response` followed by a separate call to the peak detection function (possibly with different defaults that are tuned to the harris response array). I guess the question is whether this peak detection algorithm is generally useful, or if it's somehow specific to the harris response. From the looks of code, it seems the algorithm would be generally useful. You know what: I think I'm just going to merge your Harris detection pull request as it is now, and then I'll start a new pull request with the peak detection part factored out. -Tony

On 9 January 2012 09:49, Neil Yager <yager.neil@gmail.com> wrote:

...

I think this is a great idea. Peak detection comes up all the time (e.g. peaks in Fourier space), and a general public function would be very useful.

Neil

On Jan 6, 4:59 am, Tony Yu <tsy...@gmail.com> wrote:

...

On Thu, Jan 5, 2012 at 12:50 PM, Varoquaux <

reply+i-2636374-54829e76ead08234132b3dcaac8a1c3f1441e0cd-133...@reply.github.com

...

...

wrote: I fixed a few things, and added the parameter to set the deviation

...

...

gaussian blur. I also added @GaelVaroquaux as a contributor, as he helped me debug the patch.

...

I'll now work on the panorama stitching example.

I just read through the PR for Harris corner detection<https://github.com/scikits-image/scikits-image/pull/106/files . I made some inline comments on github, but I wanted to raise a question

for the the

...

list.

It seems that everything in the `harris` function is essentially a peak detection algorithm. (Please correct me if I'm wrong or if the algorithm is specific to the Harris response.) It might be nice to make `_compute_harris_response` a public function (maybe renamed to `harris_response` or `harris`) and move the rest of `harris` to the detection subpackage as a peak detection function. We could really use a (or multiple) peak detection function(s) in skimage (e.g. with the hough transform or template matching).

-Tony