Skip to content

Commit

Permalink
Fixed bug where in place operators returned a new value
Browse files Browse the repository at this point in the history
  • Loading branch information
SamFlt committed Dec 8, 2023
1 parent b8a3ecf commit 4663d69
Show file tree
Hide file tree
Showing 2 changed files with 48 additions and 36 deletions.
78 changes: 45 additions & 33 deletions modules/python/doc/rst/python_api/conversions.rst
Original file line number Diff line number Diff line change
Expand Up @@ -41,42 +41,51 @@ You can view

To reinterpret a supported ViSP object as a Numpy array, use either:

.. doctest::
.. testcode::

>>> from visp.core import ColVector
>>> import numpy as np
from visp.core import ColVector
import numpy as np

>>> list_representation = [i for i in range(3)]
>>> vec = ColVector(list_representation) # Initialize a 3 vector from a list
>>> np_vec = vec.numpy() # A 1D numpy array of size 3
list_representation = [i for i in range(3)]
vec = ColVector(list_representation) # Initialize a 3 vector from a list
np_vec = vec.numpy() # A 1D numpy array of size 3

>>> np.all(np_vec == list_representation)
True
print(np.all(np_vec == list_representation))


vec *= 2.0
print(np.all(np_vec == list_representation))
>>> vec *= 2.0
>>> np.all(np_vec == list_representation)
.. testoutput::

True
False


or

.. doctest::
.. testcode::

>>> from visp.core import ColVector
>>> import numpy as np
from visp.core import ColVector
import numpy as np

>>> list_representation = [i for i in range(3)]
>>> vec = ColVector(list_representation) # Initialize a 3 vector from a list
>>> np_vec = np.array(vec, copy=False) # A 1D numpy array of size 3
list_representation = [i for i in range(3)]
vec = ColVector(list_representation) # Initialize a 3 vector from a list
np_vec = np.array(vec, copy=False) # A 1D numpy array of size 3

>>> np.all(np_vec == list_representation)
True
print(np.all(np_vec == list_representation))
# Modifying the ViSP vector modifies the NumPy view
vec *= 2.0
print(np.all(np_vec == list_representation))
>>> vec *= 2.0 # Modifying the ViSP vector modifies the NumPy view
>>> np.all(np_vec == list_representation)
False
# Modifying the NumPy array modifies the ViSP object
np_vec[:2] = 0.0
print(vec[0] == 0.0 and vec[1] == 0.0)

.. testoutput::

>>> np_vec[:2] = 0.0 # Modifying the NumPy array modifies the ViSP object
>>> vec[0] == 0.0 and vec[1] = 0.0
True
False
True


Expand All @@ -86,20 +95,23 @@ may lead to an invalid representation (Such as a rotation matrix not conserving

Thus, this code will not work:

.. doctest::
:options: +IGNORE_EXCEPTION_DETAIL
.. testcode::

>>> from visp.core import RotationMatrix, HomogeneousMatrix
>>> import numpy as np
from visp.core import RotationMatrix, HomogeneousMatrix
import numpy as np

R = RotationMatrix()
R.numpy()[0, 1] = 1.0

T = HomogeneousMatrix()
T.numpy()[0, 1] = 1.0

.. testoutput::
:options: +IGNORE_EXCEPTION_DETAIL

>>> R = RotationMatrix()
>>> R.numpy()[0, 1] = 1.0
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: assignment destination is read-only

>>> T = HomogeneousMatrix()
>>> T.numpy()[0, 1] = 1.0
Traceback (most recent call last):
File "<stdin>", line 1, in <module>
ValueError: assignment destination is read-only
Expand Down Expand Up @@ -139,7 +151,7 @@ For instance, the following code will lead to an undesired behaviour:
velocity = ColVector(6, 0.1)
iteration = 0
# Store the velocities in a list
log_data: List[np.ndarray] = []
log_data = []

# Servoing loop
while iteration < 10:
Expand Down
6 changes: 3 additions & 3 deletions modules/python/generator/visp_python_bindgen/header.py
Original file line number Diff line number Diff line change
Expand Up @@ -377,7 +377,7 @@ def add_method_doc_to_pyargs(method: types.Method, py_arg_strs: List[str]) -> Li
for cpp_op, python_op_name in unary_return_ops.items():
if method_name == f'operator{cpp_op}':
operator_str = f'''
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self) {{
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self) -> {return_type_str} {{
return {cpp_op}self;
}}, {", ".join(py_args)});'''
add_to_method_dict(f'__{python_op_name}__', MethodBinding(operator_str, is_static=False, is_lambda=True,
Expand All @@ -389,7 +389,7 @@ def add_method_doc_to_pyargs(method: types.Method, py_arg_strs: List[str]) -> Li
for cpp_op, python_op_name in binary_return_ops.items():
if method_name == f'operator{cpp_op}':
operator_str = f'''
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self, {params_strs[0]} o) {{
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self, {params_strs[0]} o) -> {return_type_str} {{
return (self {cpp_op} o);
}}, {", ".join(py_args)});'''
add_to_method_dict(f'__{python_op_name}__', MethodBinding(operator_str, is_static=False, is_lambda=True,
Expand All @@ -398,7 +398,7 @@ def add_method_doc_to_pyargs(method: types.Method, py_arg_strs: List[str]) -> Li
for cpp_op, python_op_name in binary_in_place_ops.items():
if method_name == f'operator{cpp_op}':
operator_str = f'''
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self, {params_strs[0]} o) {{
{python_ident}.def("__{python_op_name}__", []({"const" if method_is_const else ""} {name_cpp}& self, {params_strs[0]} o) -> {return_type_str} {{
self {cpp_op} o;
return self;
}}, {", ".join(py_args)});'''
Expand Down

0 comments on commit 4663d69

Please sign in to comment.