mujoco icon indicating copy to clipboard operation
mujoco copied to clipboard

Python bindings do not work well with type checkers

Open ikamensh opened this issue 4 years ago • 4 comments

Hello,

Problem

in the intro page of python bindings, example has following code: model = mujoco.MjModel.from_xml_string(XML, ASSETS)

It triggers a warning in pycharm type checker: "Parameter 'assets' unfilled", because as of mujoco 2.1.5 the type hint for from_xml_string thinks it's not a static method:

    def from_xml_string(self, xml, assets, Dict=None, p_str=None, bytes=None, *args, **kwargs): # real signature unknown; NOTE: unreliably restored from __doc__ 
        """
        from_xml_string(xml: str, assets: Optional[Dict[str, bytes]] = None) -> mujoco._structs.MjModel
        
        Loads an MjModel from an XML string and an optional assets dictionary.
        """
        pass

Motivation for fixing this

To avoid bugs in the code, I want to use type checkers with mujoco python bindings.


Is there a way to ship correct signatures?

ikamensh avatar Apr 15 '22 08:04 ikamensh

We've had mixed success with auto-generated type stubs for internal usage. The .pyi stubs were generated using mypy, but the tool gets confused in a few places and we have a separate script that performs specific textual replacements on the output.

Perhaps try mypy and let us know how well that works for you?

saran-t avatar Apr 21 '22 15:04 saran-t

Not sure I've installed it right, but it fails to find stubs for mujoco:

(venv) ~/P/Mujee $ mypy -c "import mujoco"                                                                                                         
<string>:1: error: Cannot find implementation or library stub for module named "mujoco"
<string>:1: note: See https://mypy.readthedocs.io/en/stable/running_mypy.html#missing-imports
Found 1 error in 1 file (checked 1 source file)

I also don't see how mypy can accept signature containing self (non-static method) being used as a static method.

ikamensh avatar Apr 22 '22 14:04 ikamensh

if you intend for type checkers to use sources for types, you should include py.typed file in the source tree. https://blog.whtsky.me/tech/2021/dont-forget-py.typed-for-your-typed-python-package/

My IDE resolves to mujoco/_structs/MjModel.py -> class MjModel when finding signature for mujoco.MjModel.from_xml_string

ikamensh avatar Apr 22 '22 14:04 ikamensh

Technically we currently don't support type checkers, so it would be inappropriate to include a py.typed. If we do add .pyi files with our future releases we'll make sure that the package is appropriately structured to enable type checking.

mypy does often get confused when it comes to static methods, however that's not what's causing the error above. You're calling mypy which is used to check type rather than to generate type stubs. You need to use stubgen instead (which comes with mypy).

saran-t avatar Apr 27 '22 13:04 saran-t