buffer_protocol for vectors and matrices in python bindings

[unjambonakap]

This is an extension/takeover of https://github.com/gazebosim/gz-math/pull/498

Summary

This change enables the buffer_protocol for VectorX and MatrixYxY so that memoryiew and numpy.array can be used on them

np.array(math7.Vector3())
memoryview(math7.Matrix3d())

list(math7.Vector2d())
>>> math7.Quaterniond().xyzw() # == [0.0, 0.0, 0.0, 1.0]

Additionally:

• IndexError is raised on invalid vector getitem/setitem access. This way list(vector) works correctly instead of spinning
• A convenience Quaternion.xyzw() -> list is added

Checklist

• [x] Signed all commits for DCO
• [x] Added tests
• [ ] Updated documentation (as needed)
• [ ] Updated migration guide (as needed)
• [x] Consider updating Python bindings (if the library has them)
• [x] codecheck passed (See contributing)
• [x] 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[bot]]

Codecov Report

Merging #524 (f1eeaf4) into gz-math7 (1e84669) will not change coverage. The diff coverage is n/a.

:exclamation: Current head f1eeaf4 differs from pull request most recent head 4907781. Consider uploading reports for the commit 4907781 to get more accurate results

@@            Coverage Diff            @@
##           gz-math7     #524   +/-   ##
=========================================
  Coverage     99.70%   99.70%           
=========================================
  Files            77       77           
  Lines          7026     7026           
=========================================
  Hits           7005     7005           
  Misses           21       21           

Impacted Files	Coverage Δ
include/gz/math/Matrix3.hh	100.00% <ø> (ø)
include/gz/math/Matrix4.hh	100.00% <ø> (ø)
include/gz/math/Matrix6.hh	100.00% <ø> (ø)
include/gz/math/Vector2.hh	100.00% <ø> (ø)
include/gz/math/Vector3.hh	100.00% <ø> (ø)
include/gz/math/Vector4.hh	100.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[unjambonakap]

Tests are failing because of lack of numpy. Do you prefer to add numpy to the test environment or introducing this dependency is undesirable and I should rewrite the test to do away with numpy?

[unjambonakap]

you can add the dependency here https://github.com/gazebosim/gz-math/blob/gz-math7/.github/ci/packages.apt

Done, thanks for the pointer :pray:

[unjambonakap]

@ahcorde do you have additional pointers on why tests are failing? In particular, homebrew is missing numpy

[mjcarroll]

do you have additional pointers on why tests are failing? In particular, homebrew is missing numpy

We generally don't add dependencies to stable releases. If you could, retarget this to main, and we can work on landing it there first with the additional dependencies.

[azeey]

We generally don't add dependencies to stable releases. If you could, retarget this to main, and we can work on landing it there first with the additional dependencies.

Agreed. Hopefully we can backport to gz-math7 without the numpy tests once it lands on main.

[willstott101]

Cool, thanks for taking this over.

Perhaps the comments could help explain a bit more what's going on/what the alternatives are?

  /// \brief Underlying data pointer
  /// \remarks This method is intended for python bindings (numpy).
  /// \remarks The bounds-checking array subscript operator is much preferred for element access from C++.
  /// \return A pointer to the underlying data array.

[arjo129]

Hi @unjambonakap , was wondering what the status on this was. Do we intend shipping this change with ionic (code freeze 26/8)? If we can't come to a consensus by then I will remove the beta label. For starters would it be possible for you to resolve the merge conflicts.

[arjo129]

Also as this PR breaks API can we plese target main and not gz-math7

[azeey]

@unjambonakap any chance you can respond to the feedback in the next couple days? Since gz-math is a lower level library in the Gazebo stack, we'd like to wrap up PRs earlier to make sure there is enough time to test them before the code freeze (August 28th).

[unjambonakap]

By Monday I should get it done

On Thu, Aug 22, 2024, 6:13 PM Addisu Z. Taddese @.***> wrote:

@unjambonakap https://github.com/unjambonakap any chance you can respond to the feedback in the next couple days? Since gz-math is a lower level library in the Gazebo stack, we'd like to wrap up PRs earlier to make sure there is enough time to test them before the code freeze (August 28th).

— Reply to this email directly, view it on GitHub https://github.com/gazebosim/gz-math/pull/524#issuecomment-2305149159, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6XYYD54UMW37YDE7L62I3ZSYE3LAVCNFSM6AAAAABMFTYQRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBVGE2DSMJVHE . You are receiving this because you were mentioned.Message ID: @.***>

[unjambonakap]

F. Sorry about that I was afk Thanks for doing it

On Fri, Aug 23, 2024, 10:55 PM Benoit Maurin @.***> wrote:

By Monday I should get it done

On Thu, Aug 22, 2024, 6:13 PM Addisu Z. Taddese @.***> wrote:

@unjambonakap https://github.com/unjambonakap any chance you can respond to the feedback in the next couple days? Since gz-math is a lower level library in the Gazebo stack, we'd like to wrap up PRs earlier to make sure there is enough time to test them before the code freeze (August 28th).

— Reply to this email directly, view it on GitHub https://github.com/gazebosim/gz-math/pull/524#issuecomment-2305149159, or unsubscribe https://github.com/notifications/unsubscribe-auth/AA6XYYD54UMW37YDE7L62I3ZSYE3LAVCNFSM6AAAAABMFTYQRCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMZDGMBVGE2DSMJVHE . You are receiving this because you were mentioned.Message ID: @.***>

[azeey]

I didn't address some of the feedback. Mainly, I think it would be good to add c++ tests.