Added JointAxis Python interface

[ahcorde]

Signed-off-by: ahcorde ahcorde@gmail.com

🎉 New feature

This PR is part of this meta ticket #931

Summary

Added JointAxis Python interface

Test it

from sdformat import JointAxis

jointAxis = JointAxis()

Checklist

• Signed all commits for DCO
• Added tests
• Added example and/or tutorial
• Updated documentation (as needed)
• Updated migration guide (as needed)
• codecheck passed (See contributing)
• All tests passed (See test coverage)
• While waiting for a review on your PR, please help review another open pull request to support the maintainers

Note to maintainers: Remember to use Squash-Merge and edit the commit message to match the pull request summary while retaining Signed-off-by messages.

[codecov-commenter]

Codecov Report

Merging #960 (7b09b64) into main (bb84733) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##             main     #960   +/-   ##
=======================================
  Coverage   66.66%   66.66%           
=======================================
  Files           2        2           
  Lines          27       27           
=======================================
  Hits           18       18           
  Misses          9        9           

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bb84733...7b09b64. Read the comment docs.

[scpeters]

it is counter-intuitive that the resolve function evaluates to True on failure (because the Errors vector is not empty) and False on success (when Errors is empty).

@azeey is there anything we can do to reverse this? Can we create a custom binding for the sdf::Errors vector that reverses this logic?

[azeey]

Since sdf::Errors is a std::vector of sdf::Error objects, I think it's automatically converted to python list of sdf::Error objects. I'm not sure if it would be possible to reverse the logic for checking if a list is empty. I think it would be easier to change the test assertion:

 def assert_no_errors(errs):
    self.assertFalse(errs)

and use assert_no_errors here.

[scpeters]

that helper would certainly make our tests more readable, but I think it would be a defect in our python API if any method that returns an sdf::Errors object would resolves to True in the presence of errors and False when there are no errors. I know that C treats a return code == 0 as success, but this isn't C. It just seems backwards to me.

if axis.resolve_xyz(vec3):
    # what do you expect here, success right?

it seems like a big usability issue to me

[ahcorde]

what about using len ?

if len(axis.resolve_xyz(vec3)) == 0:
   ...

[scpeters]

what about using len ?

if len(axis.resolve_xyz(vec3)) == 0:
   ...

that's a nice way to do it

maybe I'm overthinking it, but I think now is the time to try to fix this if we think it's worth fixing

[azeey]

It does feel backward when written that way because that feels like you would get a Result object back from that function call. For someone who expects an Errors object, reverting the logic would still lead to confusion because one might do:

errors = axis.resolve_xyz(vec3)
if errors:
  # Handle errors
  print(errors) # or something more important

I believe the pythonic way to do error handling is to use exceptions, so a potential solution would be to wrap the function so that it raises an exception if it has an error.

[scpeters]

I believe the pythonic way to do error handling is to use exceptions, so a potential solution would be to wrap the function so that it raises an exception if it has an error.

yes, this feels like a more proper way to do things. I think for the time being we can continue using a direct mapping of the API, but I'll add a bullet point to our meta-ticket suggesting error-handling with exceptions

thanks for the great suggestion!

[ahcorde]

Done here 0fae02b

[azeey]

I still see self.assertTrue(axis.resolve_xyz(vec3)). Are you planning to
change it?

[azeey]

I still see self.assertTrue(axis.resolve_xyz(vec3)). Are you planning to
change it?