dfki-ric / pytransform3d

3D transformations for Python.
https://dfki-ric.github.io/pytransform3d/
Other
615 stars 63 forks source link

TransformManager.transform_to_ij_index is a `set` but should be a `dict`. #275

Closed BartSte closed 9 months ago

BartSte commented 10 months ago

Info

Platforms: Windows & Linux Version: 3.4.0

Issue

When I run the following code:

from numpy import eye
from pytransform3d.transform_manager import TransformManager

tm: TransformManager = TransformManager()
tm.add_transform("A", "B", eye(4))

dict_ok: dict = tm.to_dict()
tm.set_transform_manager_state(dict_ok)
dict_crash: dict = tm.to_dict()

I get:

Traceback (most recent call last):
  File "/home/foo/code/./pytransform3d_minimal_example.py", line 8, in <module>
    tm_as_dict: dict = tm.to_dict()
                       ^^^^^^^^^^^^
  File "/home/foo/code/.venv/lib/python3.11/site-packages/pytransform3d/transform_manager/_transform_manager.py", line 303, in to_dict
    "transform_to_ij_index": list(self.transform_to_ij_index.items()),
                                  ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AttributeError: 'set' object has no attribute 'items'

Cause

The crash is caused by line 347 in pytransform3d/transform_manager/_transform_manager.py:

image

Here, a set object is created, instead of a dict. As a result, the program crashes when calling TransformManager.to_dict(), because TransformManager.transform_to_ij_index is of the wrong type.

Workaround

In older versions, instead of the {} in _transform_manager.py:347, dict() was used. Which version exactly, I did not look-up. Version 3.1.0 worked for me.

BartSte commented 10 months ago

I can make a PR for you if you want.

AlexanderFabisch commented 10 months ago

Hi @BartSte ,

since the unit tests seem to work for Python 3.7-11, I guess this is a Python version problem. On which Python version are you?

If you find a solution that works for all versions, please make a PR.

AlexanderFabisch commented 10 months ago

Nevermind, I think the code is broken. Probably the path is not properly tested. When you work on the PR, could you first create a unit test that crashes and then fix it? I guess with a set it wouldn't be possible to request any transformation from the TransformManager object.

BartSte commented 10 months ago

I'm on it: https://github.com/BartSte/pytransform3d/tree/fix/issue-275-transform_ij_index_is_a_set

AlexanderFabisch commented 10 months ago

Thanks for fixing the issue. I'll probably release it with 3.5.0 on the weekend.

BartSte commented 10 months ago

Nice! Thanks!