rosbridge_suite
rosbridge_suite copied to clipboard
RobotWebTools
Use pydantic instead of basic_type_check
Currently the rosbridge_library doesn't have any type hints (PEP 484). Type hints enhance the readability and maintainability of the codebase.
In the Capability class in capability.py, which all the capabilities inherit from, there is a basic_type_check function which performs type checking for the message fields the capability receives.
Instead of using this function, we can use pydantic which will 1) perform the type checking and 2) add type annotations for the message fields. If dependencies on pip packages isn't wanted, moving over to using dataclasses instead would be a better state than the current list of tuples (which will at least provide some time hints).
I think this could improve the codebase quality and would remove redundant code.
I would be happy to work in this PR :)
@sea-bass would love to hear your opinion about this one before I start refactoring stuff locally :) BTW are there any other active maintainers?
@EzraBrooks is also active, and I can almost guarantee he and I would both be very much pro type hints! See https://github.com/RobotWebTools/rosbridge_suite/pull/886/commits/b43459821d74710d46a4f571058571d82d1c4bfe for example.
I'd be happy to help review a PR for this. The serialization and deserialization in this package is pretty hard for me to wrap my head around - especially where there's special cases for certain types, which exist in a few places - and it would be great to have static type checking to help trace through it.
especially where there's special cases for certain types, which exist in a few places
Could you mention some of the places you've seen this, so I would skim through them? :)
Well, after playing with it a little I found the pydantic version that rosdep supports is at most 1.10 which is a little outdated. The newest pip version is 2.6 where breaking changes were introduced since the 1.* versions.
I'm not sure what approach I should take, either implementing a simpler version of pydantic on dataclass (which really isn't ideal), or using the older pydantic version.
What do you think would be the better approach?
here's one such odd place, where the library implicitly converts uint8[] to a base64 string. This one in particular actually bit me recently on a project.
https://github.com/RobotWebTools/rosbridge_suite/blob/7d78af16d30d0ffe232abcc65d0928ce90bd61f7/rosbridge_library/src/rosbridge_library/internal/message_conversion.py#L203
It looks like Pydantic is at version 1.10 even on Ubuntu Noble, which releases this month (April 2024), so it seems we should target the older version.