failed unittest in py-kdtree_test.py

[Jason Remillard]

Hi Paul,

I pulled your git tree (at least I think) and updated the comments.
The find_within_range is not using the manhattan distance metric, but
rather an absolute distance metric. (what I meant by _abs) function.
Nothing wrong with that, it is very fast. It just needs to be
documented. The patch changes the comment, and adds in some more
information that confused me when I first started using the library. I
also took a stab at commenting the optimize function which also
confused me when I was starting out.

I have not used git before, so hopefully the patch works.

Thanks
Jason.

On Sat, Dec 5, 2009 at 11:43 PM, Paul Harris <paulharris at computer.org> wrote:
>
>
> 2009/12/6 Jason Remillard <remillard.jason at gmail.com>
>>
>> Hi,
>>
>> I am one of the users that has been caught by the find_within_range
>> functions :-)
>>
>
> Hi Jason,
>
>>
>> Breaking peoples code is kind of drastic. A simple comment above each
>> find functions would have prevented any confusion on my part.
>>
>
> True.  My biggest concern is the welfare of those programmers who have been
> caught but are unaware.  A comment will save future users from harm, but not
> the current users.
>
> I find the biggest confusion is that you can do
> find_within_range(point,distance)
>
> which converts it into
> find_within_range( box_shaped_region )
>
> The point-distance function is deceptive, with or without a comment.  That
> is what I was hoping to kill off.
>
>
> Like you said, maybe we just need to add comments, better documentation, and
> a big "warning" on the webpage and ChangeLog or something like that.   If a
> developer upgrades without checking what changes were made, then ... their
> loss I guess.
>
>
>> Just add in the new functions (I assume they look something like this)
>>
>> find_within_range_abs
>
> I don't understand what this function would do... ?
> Any distance measurement must be absolute, no?   Unless you are searching
> based on a point+direction (ie a vector) or something funky like that.
>
>> find_within_range_manhattan
>> find_within_range_euclidean
>>
>
> These I understand, and I like the naming (although "manhattan" is known by
> many names like "taxi distance" or "city block" etc etc).
>
>>
>> Just leave find_within_range in place, and have it be a simple one
>> line function calling one of the new functions. It is pretty much
>> impossible to use the class without looking in the header file, so it
>> will be crystal clear to everybody what is going on when using the new
>> release. Now if the plan is to not support abs distance, then you
>> should kill the function rather than changing the systematics.
>>
>
> thanks for your comments :)
> Paul
>
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0001-Add-more-commands.patch
Type: application/octet-stream
Size: 2521 bytes
Desc: not available
URL: <http://lists.alioth.debian.org/pipermail/libkdtree-devel/attachments/20091206/58bcf521/attachment.obj>