Remove deprecated numpy.matrix (re: issue 524)

[ScottMonaghan]

@ijnek, if you have a chance, please review my progress and let me know what you'd like to see done differently.

These are the fixes for:

• 524

Done so far:

MUST HAVE

• [x] No supported API members result in a numpy.matrix() call
• [x] No supported API members have a return type of numpy.matrix
• [x] API documentation must be updated to reflect any changes
• [x] Tests updated to reflect any changes & passing

SHOULD HAVE

• To prevent breaking changes to API:
  • [x] A new supported API member that returns numpy.ndarray should be created for each currently supported API member that currently returns numpy.matrix
  • [x] Existing API members that return numpy.matrix object generate a DeprecationWarning and suggest replacement method when called, and note changed return type of new methods
• General code cleanup
  • [x] Update StereoCameraModel to only call public members of PinholeCameraModel, rather than private K,D,P,ect. directly.
  • [x] Update to PEP8 Guidelines where possible

NICE TO HAVE

• [x] New api methods should use snake_case to fit PEP8 Guidelines.
• [x] Deprecate the rest of the camelCase public API methods with a warning and replace with snake_case
  • The justification here is that we don't know when this piece of code will be touched again and so it's my preference to bring it up to standard as much as possible.

[ScottMonaghan]

@ijnek, I was able to get to this a lot quicker than expected (thanks to being sick in bed with the flu).

Outside of any fixes you'd like to see, I have two outstanding concerns we may want to address before tying this thing off:

1. The official ROS 2 documentation for image_geometry only builds for C++ and not for Python. While I've successfully built the sphinx documentation for the python libraries locally, I'm not sure what needs to be done to make sure the docs get built for the official package repo: http://docs.ros.org/en/rolling/p/image_geometry/index.html#module-image_geometry
2. The directed.py unit tests do not get run by colcon test. I've been running them manually to ensure they all work, but I'm not sure what is needed here.

I'm also not sure what this is about:

Build error. I don't think anything I touched could have caused this, but I could be wrong:

13:21:44 ERROR: Step ‘Publish xUnit test result report’ failed: Test reports were found but not all of them are new. Did all the tests run?
13:21:44   * /home/jenkins-agent/workspace/Rpr__vision_opencv__ubuntu_noble_amd64/ws/test_results/cv_bridge/Testing/20240406-0335/Test.xml is 14 hr old
13:21:44   * /home/jenkins-agent/workspace/Rpr__vision_opencv__ubuntu_noble_amd64/ws/test_results/image_geometry/Testing/20240406-0335/Test.xml is 14 hr old
13:21:44 
13:21:44 Setting status of 5e2a5a188454ff2f3f49177956cd1b16c8ff9ab5 to FAILURE with url https://build.ros2.org/job/Rpr__vision_opencv__ubuntu_noble_amd64/4/ and message: 'Build finished. '
13:21:44 Using context: Rpr__vision_opencv__ubuntu_noble_amd64
13:21:44 Finished: FAILURE

[ijnek]

Hi Scott, thanks for taking on these changes! I have a few suggestions to ensure smooth transition for downstream packages. Please tell me your thoughts:

• Add deprecation decorators to the deprecated methods, mentioning when method is going to be removed:

  @deprecation.deprecated(details="The FooBar() method which returns a numpy.matrix is deprecated as of J-turtle, and will be removed in K-turtle. Please use the foo_bar() method which returns a numpy.ndarray instead.")

• Add deprecated @attribute methods to handle cases where down-stream users were directly accessing the private variables in their existing code:

  @property
  @deprecation.deprecated(details="...")
  def K(self):
      return numpy.asmatrix(self._k)

[ijnek]

In regards to the buildfarm failure, this is most likely related to instability of the apt packages due to the ubuntu noble transition. I see these dependencies being correctly installed in more recent pr build, so I'm hoping you won't see the build fail on your next commit.

Investigating (0) libopencv-videoio406:amd64 < none -> 4.6.0+dfsg-13build5 @un puN Ib >
Broken libopencv-videoio406:amd64 Depends on libavcodec60:amd64 < none @un H > (>= 7:6.0)
  Considering libavcodec60:amd64 0 as a solution to libopencv-videoio406:amd64 8
  Considering libavcodec-extra60:amd64 0 as a solution to libopencv-videoio406:amd64 8
Broken libopencv-videoio406:amd64 Depends on libavformat60:amd64 < none @un H > (>= 7:6.0)
  Considering libavformat60:amd64 0 as a solution to libopencv-videoio406:amd64 8
  Considering libavformat-extra60:amd64 0 as a solution to libopencv-videoio406:amd64 8
Investigating (0) libavcodec-dev:amd64 < none -> 7:6.1.1-1ubuntu1 @un puN Ib >
Broken libavcodec-dev:amd64 Depends on libavcodec60:amd64 < none @un H > (= 7:6.1.1-1ubuntu1)
  Considering libavcodec60:amd64 0 as a solution to libavcodec-dev:amd64 4
  Considering libavcodec-extra60:amd64 0 as a solution to libavcodec-dev:amd64 4
Investigating (0) libavformat-dev:amd64 < none -> 7:6.1.1-1ubuntu1 @un puN Ib >
Broken libavformat-dev:amd64 Depends on libavformat60:amd64 < none @un H > (= 7:6.1.1-1ubuntu1)
  Considering libavformat60:amd64 0 as a solution to libavformat-dev:amd64 3
  Considering libavformat-extra60:amd64 0 as a solution to libavformat-dev:amd64 3
Done

[ijnek]

Outside of any fixes you'd like to see, I have two outstanding concerns we may want to address before tying this thing off:

1. The official ROS 2 documentation for image_geometry only builds for C++ and not for Python. While I've successfully built the sphinx documentation for the python libraries locally, I'm not sure what needs to be done to make sure the docs get built for the official package repo: http://docs.ros.org/en/rolling/p/image_geometry/index.html#module-image_geometry
2. The directed.py unit tests do not get run by colcon test. I've been running them manually to ensure they all work, but I'm not sure what is needed here.

For 1, to auto-generate API docs onto docs.ros.org, there is a nice guide that came out recently. I haven't tried it out myself, but I believe it applies also for packages with C++ and python. This work should belong in a separate issue / PR.

For 2, the issue seems to be that image_geometry/test/CMakeLists.txt is not used in the package's cmake file. In image_geometry/CMakeLists.txt, change this:

if(BUILD_TESTING)
  find_package(ament_cmake_gtest REQUIRED)
  ament_add_gtest(${PROJECT_NAME}-utest test/utest.cpp)
  target_link_libraries(${PROJECT_NAME}-utest ${PROJECT_NAME})
endif()

to

if(BUILD_TESTING)
  add_subdirectory(test)
endif()

[ScottMonaghan]

Thanks @ijnek.

I'll try to make some progress on this over the weekend.

[ScottMonaghan]

@ijnek, ready for review:

• [x] switched to @deprecated decorators (added the Deprecated library)
• [x] added deprecated properties for all previously exposed private members
• [x] got directed.py test working with colcon test (thanks to your tip)
• [x] cleaned up cameramodels.py, moving all deprecated members to the bottom of their respective class in alphabetical order for readability
• [x] added tests for all deprecated items + missing test for rectify_image

[ScottMonaghan]

Thanks @ijnek! I'm really excited and proud to make a small but significant contribution to the ROS 2 project!

I'd like to continue and also not leave outstanding things hanging discovered during this PR.

Would following up with a new issue and working on documentation generation be helpful?

[ijnek]

Congratulations on your first ROS contribution! Yes, that would be a contribution that would be greatly appreciated.