fzi-forschungszentrum-informatik / Lanelet2

Map handling framework for automated driving
BSD 3-Clause "New" or "Revised" License
792 stars 325 forks source link

Improve ArcCoordinates interface #351

Closed johannes-fischer closed 1 month ago

johannes-fischer commented 1 month ago

Adds another constructor, __eq__ and __repr__ functions to ArcCoordinates to make it more usable.

johannes-fischer commented 1 month ago

I first had moved the ArcCoordinates interface definition from python_api/geometry.cpp to python_api/core.cpp in https://github.com/fzi-forschungszentrum-informatik/Lanelet2/pull/351/commits/3e8ffc419fc5185b8c74de8916871ecbd8e35ff4, because in the c++ code ArcCoordinates is defined in lanelet2_core/primitives/Point.h and generally it seems that stuff from lanelet2_core/primitives is put into python_api/core.cpp whereas lanelet2_core/geometry goes into python_api/geometry.cpp. But since this might a breaking change in the python api I reverted it. Do you have an opinion on this?

poggenhans commented 1 month ago

I first had moved the ArcCoordinates interface definition from python_api/geometry.cpp to python_api/core.cpp

Do you see a good reason to move it? Other than for symmetry to C++? Imho the organization of things in python doesn't need to match the C++ organization. In C++ there are a few more constrains that won't allow us to organize things in the way we can do it in python (e.g. by using a separate geometry module). And since ArcCoordinates only makes sense in combination with the geometry module, I chose to put it there.

johannes-fischer commented 1 month ago

The symmetry to C++ was my only reason.