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

Fix convention of signed angles #438

Open
willGraham01 opened this issue Feb 25, 2025 · 1 comment · May be fixed by #445
Open

Fix convention of signed angles #438

willGraham01 opened this issue Feb 25, 2025 · 1 comment · May be fixed by #445
Labels
enhancement New optional feature

Comments

@willGraham01
Copy link
Contributor

Is your feature request related to a problem? Please describe.
First reported here.

Several of our methods that return signed angles, notably

  • compute_forward_vector_angle
  • compute_{ego,allo}centric_angle_to_nearest_point

provide a toggle for the sign convention of the returned angle(s). However the effect is usually just a sign reverse, so for the sake of simplifying the user interface and providing concrete concepts and terminology, we should fix the convention we're using for the signed angle and not provide this toggle in our functions.

NOTE:
The v_as_left_opperand argument to compute_signed_angle_2d might also be able to be removed, though this function is asymmetric in u and v, so care should be taken to check if this can be done. The function docstring contains some more elaboration.

@willGraham01 willGraham01 added the enhancement New optional feature label Feb 25, 2025
@willGraham01 willGraham01 changed the title Fix covnention of signed angles Fix covention of signed angles Feb 25, 2025
@willGraham01 willGraham01 changed the title Fix covention of signed angles Fix convention of signed angles Feb 25, 2025
@niksirbi
Copy link
Member

The v_as_left_opperand argument to compute_signed_angle_2d might also be able to be removed, though this function is asymmetric in u and v, so care should be taken to check if this can be done. The function docstring contains some more elaboration.

Yeah we might decide that it's worth keeping that option in there. As this is a lower-level utility, doesn't matter as much.

For the other 3 functions you mentioned, which are user-facing, I think we can and should get rid of the toggle.

@willGraham01 willGraham01 linked a pull request Feb 27, 2025 that will close this issue
7 tasks
@willGraham01 willGraham01 moved this from 🤔 Triage to 🚧 In Progress in movement progress tracker Feb 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New optional feature
Projects
Status: 🚧 In Progress
Development

Successfully merging a pull request may close this issue.

2 participants