Skip to content
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

Distance method of the Ray returns wrong results. #2950

Closed
wants to merge 1 commit into from

Conversation

akshayah3
Copy link
Contributor

Consider this:
Ray((1,1), (2,2)).distance(Point(1.5, 3)) gives sqrt(17)/2
but the result should be 3*sqrt(2)/4

@akshayah3
Copy link
Contributor Author

@smichr Could you take a look at this?

if self.contains(non_o):
return self.distance(o) # = s.length but simpler
return s.length
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would leave this line alone: it says that calculating distance to o is simple than computing the length of s. Did you find otherwise?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That leads to an infinite recursion.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah - it should be Line(self).distance(o)

>>> from sympy import *
>>> Ray((1,1), (2,2)).distance(Point(1.5, 3))
3*sqrt(2)/4

@smichr
Copy link
Member

smichr commented Feb 23, 2014

Good catch.

@akshayah3
Copy link
Contributor Author

@smichr So I guess you could merge this?

@smichr
Copy link
Member

smichr commented Feb 23, 2014

I made the change here, added the test and committed the result. Thanks for your improvements to geometry.

@smichr smichr closed this Feb 23, 2014
@akshayah3 akshayah3 deleted the Ray_distance branch February 23, 2014 14:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants