Thanks for the input, Tony and Stefan.  Unless someone else wants to tackle this, I'll see what I can put together over a week or so.  

Josh


On Tuesday, November 13, 2012 7:26:58 PM UTC-6, Tony S Yu wrote:


On Tue, Nov 13, 2012 at 3:58 PM, Stéfan van der Walt <ste...@sun.ac.za> wrote:
On Mon, Nov 12, 2012 at 3:57 PM, Josh Warner <silvertr...@gmail.com> wrote:
> In contrast, `is_local_maximum` has a much simpler API.  It doesn't have the

Just for the record, `is_local_maximum` is mentioned in:

http://shop.oreilly.com/product/0636920020219.do

So, if we could write a unified backend but still expose this
function, that'd be good!

Stéfan

 

I'm also in favor of keeping `is_local_maximum` and `peak_local_max` as separate functions, primarily because they have different return values (both of which have valid use cases). But... I'd be in favor of deprecating the current `is_local_maximum` in the `morphology` subpackage and renaming it to `is_local_max` in the `feature` subpackage. Unfortunately emoving the current function would break code (although the transition could be smooth if it's removed after a couple of releases with a deprecation warning). What do you think?


On Tue, Nov 13, 2012 at 12:35 AM, Josh Warner <silvertr...@gmail.com> wrote:
I can probably put something together.  What should the goal be?  Expand the featureset of one algorithm, such that the other can be collapsed into a wrapper function with no loss of backwards compatibility, or expand the featureset of one and eliminate the other (carefully changing all internal references to the old function)?  

The latter might be the best/ideal world solution, but even if all of the internal references were changed appropriately it could break 3rd party code.  I would lean toward the former option, moving in the direction of `is_local_maximum`, though this does appear to be the slower algorithm at present. 

As mentioned above, I think it'd be best to have a single core function, but keep the other function (though possibly renamed and relocated) as a wrapper of the core function. If `is_local_maximum` is slower, I think it would be good to start with `peak_local_max` as the base.

As a first pass, it'd be great to fix the border issue with `peak_local_max`. (It might be nice to make this optional, since a user may want to require that the peak must be `min_distance` away from the image border---I could go either way on whether or not this is optional). Adding masking and a footprint parameter would be great, and I assume it should be straightforward (note that `scipy.ndimage.maximum_filter` has a footprint parameter).

Best,
-Tony