AxisAlignedBox: deprecate unimplemented methods

[ahcorde]

Signed-off-by: ahcorde [email protected]

🦟 Bug fix

Fixes #

Summary

Clean some unimplemented methods in AxisAlignedBox

Checklist

• Signed all commits for DCO
• Added tests
• 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

[codecov]

Codecov Report

Merging #261 (67446f5) into ign-math6 (23bf73a) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff             @@
##           ign-math6     #261   +/-   ##
==========================================
  Coverage      99.41%   99.41%           
==========================================
  Files             67       67           
  Lines           6374     6374           
==========================================
  Hits            6337     6337           
  Misses            37       37           

Impacted Files	Coverage Δ
include/ignition/math/AxisAlignedBox.hh	100.00% <ø> (ø)

---

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 23bf73a...67446f5. Read the comment docs.

[chapulina]

This time the ABI checker complained, probably because the class is not header-only:

https://build.osrfoundation.org/job/ignition_math-abichecker-any_to_any-ubuntu_auto-amd64/616/API_5fABI_20report/

[ahcorde]

what do you think @scpeters about this PR ?

[scpeters]

I feel like we should be able to do this, so I'm not sure why the ABI checker complains

cc @j-rivero any ideas?

[j-rivero]

I feel like we should be able to do this, so I'm not sure why the ABI checker complains

cc @j-rivero any ideas?

If I understand this correctly, we are removing function declarations from a public header installed in the user system which don't have a definition, is this right? This would explain why the abi-checker is happy with respect to ABI but raise warnings in the "Source compatibility" (API).

Removed Symbols  5 
AxisAlignedBox.hh
namespace ignition::math::v6
AxisAlignedBox::DensityFromMass ( double const _mass ) const
_ZNK8ignition4math2v614AxisAlignedBox15DensityFromMassEd

AxisAlignedBox::MassMatrix ( MassMatrix3d& _massMat ) const
_ZNK8ignition4math2v614AxisAlignedBox10MassMatrixERNS1_11MassMatrix3IdEE

AxisAlignedBox::Material ( ) const
_ZNK8ignition4math2v614AxisAlignedBox8MaterialEv

AxisAlignedBox::SetDensityFromMass ( double const _mass )
_ZN8ignition4math2v614AxisAlignedBox18SetDensityFromMassEd

AxisAlignedBox::SetMaterial ( Material const& _mat )
_ZN8ignition4math2v614AxisAlignedBox11SetMaterialERKNS1_8MaterialE

It's quite a corner case and it's not easy to find a decent use-case to go against the removal but someone could be using these declarations and implementing these methods in their own code ... does not sound very convenient but it's possible and in the case, we are breaking the API for them.

Unless we really need this, my proposal is to move to the next major version.

[scpeters]

I think it would be fair to mark these APIs as deprecated

[ahcorde]

should I target main?

[j-rivero]

I think is fine target this to mah6 and release the deprecation in a MINOR due to the weird nature of the unimplemented methods. Up to maintainers but I don't see to much drama.

[scpeters]

I think is fine target this to mah6 and release the deprecation in a MINOR due to the weird nature of the unimplemented methods. Up to maintainers but I don't see to much drama.

agreed, we can deprecate these in ign-math6 because they are weird

ABI job is fixed

[osrf-triage]

This pull request has been mentioned on Gazebo Community. There might be relevant details there:

https://community.gazebosim.org/t/new-ignition-releases-2022-03-01-citadel-edifice-fortress/1313/1