-
-
Notifications
You must be signed in to change notification settings - Fork 293
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
g.gui.iclass: replace removed dist_point_to_segment() #645
g.gui.iclass: replace removed dist_point_to_segment() #645
Conversation
gui/wxpython/iscatt/plots.py
Outdated
s0 = np.asarray(s0, float) | ||
s1 = np.asarray(s1, float) | ||
|
||
return np.cross(s1-s0, p-s0)/np.linalg.norm(s1-s0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you! Could you please fix the whitespace (add space between operators), the flake8 checks are not enforcing it now, but probably in the future.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
updated, hope I got it right :)
892183b
to
a9d6dcc
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the test, just be careful, at the end you are printing the same value in both cases.
gui/wxpython/iscatt/plots.py
Outdated
s0 = np.asarray(s0, float) | ||
s1 = np.asarray(s1, float) | ||
|
||
return np.cross(s1 - s0, p - s0) / np.linalg.norm(s1 - s0) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It can return negative numbers, so you need np.abs(...), that's mentioned also in the SO answer.
Whoa, what a blunder! Thanks for noting! |
Now, having the test file corrected I did some testing of some other algorithms (and updated the code in first comment). It turned out that the SO solution wasn't as reliable as it was short and neat.
Any thoughts? |
The short one is just solving a different problem (point from line, vs point from segment). I suggest using the algorithm from matplotlib, since it was there before, unless you found some issues with it. I am not entirely sure how the licensing works, but there is pseudo code source linked to it too, so I wouldn't consider it a problem. |
I have no strong opinion on which method, matplotlib is good for me. I guess a reference of origin would suffice. I couldn't find in code base any other use of this function... shall I put it in |
Not really, I would suggest creating a new file iscatt/utils.py. |
from matplotlib, which was disabled for matplotlib 3.1.0+. Fixes OSGeo#461
a9d6dcc
to
84f2208
Compare
Updated as you suggested, please take a look. |
Thank you for getting this done! |
This replaces dist_point_to_segment() from matplotlib, which was disabled for matplotlib 3.1.0+.
Addresses: #461.
To test the algorithm [updated]: