epfl-lasa / control-libraries

A collection of library modules to facilitate the creation of full control loop algorithms, including state representation, motion planning, kinematics, dynamics and control.
https://epfl-lasa.github.io/control-libraries
GNU General Public License v3.0
27 stars 2 forks source link

Python binding of CartesianState uses argument name "reference", not "reference_frame" #308

Closed eeberhard closed 1 year ago

eeberhard commented 2 years ago

This is a minor issue discovered by @buschbapti. In C++, the constructors of CartesianState and its derived classes take a string argument called "reference_frame". However, in the Python bindings for these classes, the argument name was shortened to just "reference". This leads to the following behavior:

state_representation.CartesianPose(name="foo", reference_frame="bar")  # does not work!
state_representation.CartesianPose(name="foo", reference="bar")  # does work

Of course, the simple solution is to just treat these like the positional arguments that they are intended to be, instead of referring to them by name.

state_representation.CartesianPose("foo", "bar")  # works, and is the intended usage

Still, for the sake of the docstrings and for consistency against the C++ API, the argument name should be updated. It's a small change in the source code, but would still constitute a breaking change for any places where the keyword argument reference was used in Python usage of CartesianState and derived classes.

eeberhard commented 1 year ago

Resolved by #309. The following usage rules now apply:

state_representation.CartesianPose("foo", "bar")  # works, and is the intended usage
state_representation.CartesianPose(name="foo", reference_frame="bar")  # also works

state_representation.CartesianPose(name="foo", reference="bar")  # no longer works