From 4663d693194c5b1549a27296eb83c114d8381290 Mon Sep 17 00:00:00 2001 From: Samuel Felton Date: Fri, 8 Dec 2023 15:20:14 +0100 Subject: [PATCH] Fixed bug where in place operators returned a new value --- .../python/doc/rst/python_api/conversions.rst | 78 +++++++++++-------- .../generator/visp_python_bindgen/header.py | 6 +- 2 files changed, 48 insertions(+), 36 deletions(-) diff --git a/modules/python/doc/rst/python_api/conversions.rst b/modules/python/doc/rst/python_api/conversions.rst index acdf5f5116..992cadf0d6 100644 --- a/modules/python/doc/rst/python_api/conversions.rst +++ b/modules/python/doc/rst/python_api/conversions.rst @@ -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 @@ -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 "", line 1, in ValueError: assignment destination is read-only - - >>> T = HomogeneousMatrix() - >>> T.numpy()[0, 1] = 1.0 Traceback (most recent call last): File "", line 1, in ValueError: assignment destination is read-only @@ -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: diff --git a/modules/python/generator/visp_python_bindgen/header.py b/modules/python/generator/visp_python_bindgen/header.py index 90d00b10cb..d9e9e9da0a 100644 --- a/modules/python/generator/visp_python_bindgen/header.py +++ b/modules/python/generator/visp_python_bindgen/header.py @@ -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, @@ -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, @@ -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)});'''