jgerstmayr / EXUDYN

Multibody Dynamics Simulation: Rigid and flexible multibody systems
https://www.youtube.com/channel/UCsQD2bIPBXB_4J23WtqKkVw/playlists
Other
173 stars 23 forks source link

Wrong type of pyObject argument #64

Closed ManuelZ closed 7 months ago

ManuelZ commented 7 months ago

AddMarker supposedly receives a dict:

Source

@overload
    def AddMarker(self, pyObject: dict) -> MarkerIndex: ...

But it really receives a Python object:

Source

.def("AddMarker", &MainSystem::AddMainMarkerPyClass, "add a marker...", py::arg("pyObject"))

Source

//! Add a MainMarker with a python class
MarkerIndex AddMainMarkerPyClass(const py::object& pyObject);

Indeed, there exists a method that adds a marker when passed a dict, but when I search for it in the codebase, I don't see it being used.

Source

//! this is the hook to the object factory, handling all kinds of objects, nodes, ...
Index AddMainMarker(const py::dict& d);

The same seems to happen with AddObject, AddNode, AddLoad and AddSensor.

This is the typing error that I'm seeing:

Argument of type "MarkerBodyRigid" cannot be assigned to parameter "pyObject" of type "dict[Unknown, Unknown]" in function "AddMarker"
  "MarkerBodyRigid" is incompatible with "dict[Unknown, Unknown]"Pylance[reportArgumentType]

I guess a better type could be the object type.

jgerstmayr commented 7 months ago

This may look like a confusion, but is done by design. The interface requires a dictionary. However, usually, you are not creating the dictionary, but you are using an interface class such as ObjectRigidBody. This class provides a conversion to a dictionary, which includes type checks and alleviates the interfacing between Python and C++. In order to catch errors of the users easier when they provide something else than a dictionary, we take the most general py::object which then can give a "readable" error message.

I understand that this may confuse the very interested reader of the code, but this is done by design. So, I will change the type hint to py::object which should resolve the typing errors.

jgerstmayr commented 7 months ago

A short request: could you try to change pyObject: dict into pyObject: Any in the .pyi file of your site-packages for a trial? This should resolve the issue and it would not cause a major confusion for the user, as users should know what args to use in AddObject, AddNode, etc.

If this works for PyLance (and on my local version), I would just change this in the repo ...

ManuelZ commented 7 months ago

I've tested it and it works when I replace the types and make them Any in exudyn\__init__.pyi.

jgerstmayr commented 7 months ago

ok, will be done within version 1.8.19. Thanks for your help!

ManuelZ commented 6 months ago

Thanks. You're welcome!