Skip to content

Conversation

ghanning
Copy link
Contributor

@ghanning ghanning commented Dec 18, 2024

This pull request includes several changes to the pybind/pyposelib.cc file, primarily focusing on refactoring function signatures to use Camera objects directly instead of py::dict for camera parameters. Additionally, it introduces overloaded functions to maintain compatibility with the previous interface.

Refactoring for improved type safety:

  • Modified estimate_absolute_pose_wrapper, refine_absolute_pose_wrapper, estimate_absolute_pose_pnpl_wrapper, and refine_absolute_pose_pnpl_wrapper to accept Camera objects directly instead of py::dict for camera parameters. [1] [2] [3] [4]
  • Updated estimate_generalized_absolute_pose_wrapper and refine_generalized_absolute_pose_wrapper to use Camera objects instead of py::dict for camera parameters. [1] [2]

Maintaining backward compatibility:

  • Added overloaded versions of estimate_absolute_pose_wrapper, refine_absolute_pose_wrapper, estimate_absolute_pose_pnpl_wrapper, and refine_absolute_pose_pnpl_wrapper that convert py::dict to Camera objects internally. [1] [2] [3] [4]
  • Introduced overloaded functions for estimate_generalized_absolute_pose_wrapper and refine_generalized_absolute_pose_wrapper to handle py::dict conversions. [1] [2]

These changes improve type safety and maintain backward compatibility, ensuring a smooth transition to the new interface.

Copy link
Collaborator

@vlarsson vlarsson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@pablospe pablospe changed the title Add overloaded wrapper methods for passing poselib::Camera instead of… Add overloaded wrapper methods for passing poselib::Camera instead of py::dict. Dec 18, 2024
@vlarsson vlarsson merged commit 4bc4721 into PoseLib:master Dec 19, 2024
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants