-
Notifications
You must be signed in to change notification settings - Fork 11
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
143 expand linexy constructors and slope information #150
143 expand linexy constructors and slope information #150
Conversation
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.
Looks good! I left one suggestion for extra documentation. Feel free to take or leave it.
""" | ||
Get a new instance of this class built from the rho + theta representation of a line. | ||
|
||
Parameters | ||
---------- | ||
rho : float | ||
The right angle distance between the line and the origin (0,0). For | ||
images, this will be the top-left corner of the image. | ||
theta : float | ||
The angle of the line, in radians, for the standard graphing | ||
coordinate system. 0 is on the positive x-axis to the right, and the | ||
angle increases counter-clockwise. | ||
""" |
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.
Can you give an example here or clarify. For example, if you had rho=1 and theta=0, would the line pass through x=+1 or x=-1?
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.
I'm glad you left a comment. I double-checked the method, and realized I had both a bug and a bad definition.
- Changed the definition of theta from "angle of the line" to "angle between the x-axis and the right angle distance vector".
- Removed the cast to an integer, which was causing numeric issues.
- Added unit tests for from_rho_theta() and from_location_angle().
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.
Looks good!
…ifference between positive and negative infinity in the slope
fd22626
to
2c42308
Compare
No description provided.