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

camera_calibration tests call deprecated image_geometry members #966

Closed
2 tasks
ScottMonaghan opened this issue Apr 20, 2024 · 6 comments
Closed
2 tasks
Assignees
Labels

Comments

@ScottMonaghan
Copy link
Contributor

This issue is a follow-up to

Following the thread to fix #572 above, we overhauled the image_geometry package to both to internally deprecate any use of the np.matrix class, and also to bring the package into compliance with ROS 2 code standards.

Those changes introduced the deprecation warnings referencing fromCameraInfo() and projectPixelTo3d() below.

I'll go ahead and start working on a fix for these.

Here is my MUST, SHOULD, OUT OF SCOPE acceptance criteria for changes I think are appropriate.

@vrabaud, @JWhitleyWork, @jacobperron, @mikeferguson, please let me know if you'd like me to take a look at anything else while I'm in there.

MUST HAVE

  • camera_calibration test does not raise deprecation warnings

SHOULD HAVE

  • camera_calibration python scripts do not reference any deprecated image_geometry members

OUT OF SCOPE

  • disutils package warning below (this should be a separate issue)

============================= warnings summary ===============================
src/camera_calibration/calibrator.py:47
Warning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives

test/test_directed.py::TestDirected::test_stereo
test/test_directed.py::TestDirected::test_stereo
test/test_directed.py::TestDirected::test_stereo
test/test_directed.py::TestDirected::test_stereo
Warning: Call to deprecated method fromCameraInfo. (The fromCameraInfo() method is deprecated as of J-turtle, and will be removed in K-turtle. Please use the from_camera_info() method instead.) -- Deprecated since version J-turtle.

test/test_directed.py: 192 warnings
Warning: Call to deprecated method projectPixelTo3d. (The projectPixelTo3d() method is deprecated as of J-turtle, and will be removed in K-turtle. Please use the project_pixel_to_3d() method instead.) -- Deprecated since version J-turtle.

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html

Finished <<< camera_calibration [31.7s]

Summary: 1 package finished [32.1s]
1 package had stderr output: camera_calibration

@ScottMonaghan ScottMonaghan changed the title camera_calibration tests call deprecated image_geometry methods camera_calibration tests call deprecated image_geometry members Apr 20, 2024
@ScottMonaghan
Copy link
Contributor Author

UPDATE: it looks like the updated image_geometry 4.1.0 package hasn't migrated yet to packages.ros.org, so I'll wait to make any changes to camera_calibration until the package has been officially updated.

image

Question: it looks like packages.ros.org was last updated at 11pm on Thursday 18-April. Is this something that happens weekly on Thursdays, or is there a different cadence?

@ahcorde
Copy link
Contributor

ahcorde commented Apr 22, 2024

Question: it looks like packages.ros.org was last updated at 11pm on Thursday 18-April. Is this something that happens weekly on Thursdays, or is there a different cadence?

ROS 2 releases are announced in ROS discourse and in general there is a 3-4 weeks cadence. You can take a look here

@ScottMonaghan
Copy link
Contributor Author

I have the fixes ready that are compatible with the released image_geometry 4.1 package, but the package isn't live on packages.ros.org yet, and the changes would be breaking otherwise. Should I create the PR or hold off until 4.1 is available through rosdep & apt?

@ahcorde
Copy link
Contributor

ahcorde commented Apr 22, 2024

@ScottMonaghan you can create the PR, I will run CI using the sources.

@ScottMonaghan
Copy link
Contributor Author

Fix submitted in PR #968.

Once that is merged, this issue can be closed.

@ScottMonaghan
Copy link
Contributor Author

Now that #968 is merged. This can be closed.

ahcorde pushed a commit that referenced this issue May 8, 2024
This is a PR to fix: 
- #966 

As noted in #966, as of writing image_pipeline [4.1.0 has been
released](https://github.com/ros-perception/vision_opencv/releases/tag/4.1.0),
is updated on
[index.ros.org](https://index.ros.org/p/image_geometry/github-ros-perception-vision_opencv/#rolling),
but it has not yet been migrated to
[packages.ros.org](http://packages.ros.org/ros2/ubuntu/dists/noble/main/binary-amd64/Packages).

As such `camera_calibration` will also require the source of
[image_pipeline
4.1.0](https://github.com/ros-perception/vision_opencv/releases/tag/4.1.0)
or higher to successfully build.

I tested to ensure successful build with colcon build & colcon test.

Note that colcon test has the following warning that is out of scope of
this PR:
```
=============================== warnings summary ===============================

src/camera_calibration/calibrator.py:47

  Warning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
```
Please let me know if there are any questions, concerns, or requested
changes.
Kotochleb pushed a commit to Kotochleb/image_pipeline that referenced this issue May 27, 2024
This is a PR to fix: 
- ros-perception#966 

As noted in ros-perception#966, as of writing image_pipeline [4.1.0 has been
released](https://github.com/ros-perception/vision_opencv/releases/tag/4.1.0),
is updated on
[index.ros.org](https://index.ros.org/p/image_geometry/github-ros-perception-vision_opencv/#rolling),
but it has not yet been migrated to
[packages.ros.org](http://packages.ros.org/ros2/ubuntu/dists/noble/main/binary-amd64/Packages).

As such `camera_calibration` will also require the source of
[image_pipeline
4.1.0](https://github.com/ros-perception/vision_opencv/releases/tag/4.1.0)
or higher to successfully build.

I tested to ensure successful build with colcon build & colcon test.

Note that colcon test has the following warning that is out of scope of
this PR:
```
=============================== warnings summary ===============================

src/camera_calibration/calibrator.py:47

  Warning: The distutils package is deprecated and slated for removal in Python 3.12. Use setuptools or check PEP 632 for potential alternatives
```
Please let me know if there are any questions, concerns, or requested
changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants