-
-
Notifications
You must be signed in to change notification settings - Fork 1.7k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Type annotations for Python API #2385
Comments
We use swig to generate the Python API from the C++ API and there is no easy way to introduce type annotations there, see swig/swig#735 for a discussion on this topic. So, you local stubs approach may be a better way. I guess we shall it to the Python library build process somewhere here: https://github.com/cyberbotics/webots/tree/master/resources/languages/python Alternatively, it shouldn't be that difficult to get rid of SWIG for the Python wrapper and maintain it, based on the C libController library (instead of the C++ one). But that's probably a bit more of work, but could be also very nice and fix other problems caused by SWIG. |
If we can't figure out a way to (1) make swig automatically generate appropriate python type hints (and my sense from reading threads like the one @omichel linked is that this isn't really feasible yet), I suppose another option would be (2) to write a script that automatically adds appropriate type hints to the swig output. Writing Python code to alter Python code is quite straightforward, using Python Abstract Syntax Trees (AST's). The main challenge would just be to figure out which types to assign to which arguments and outputs. This mapping could be constructed manually, and would likely require little maintenance afterwards, or there may be a way to get swig to generate its auto-documentation (which apparently will be wrong in various ways, like using C++ names for the types rather than python names) and then convert that into the appropriate python type hints. On the bright side, Python type hints and documentation are optional, so this needn't be perfect or complete in order to be useful. So, e.g., a partial solution that adds type hints to commonly used things would be very useful, and it wouldn't really matter very much if newly added Webots features don't immediately get their own type hints. A related useful feature, which might be good to fold in with this, would be to include (either a hyperlink to or a copy of) Webots documentation in the Python docstrings for functions in controller.py, so that this documentation could be immediately available within a Python IDE like PyCharm, rather than requiring flipping over to a browser and searching there. |
All what you propose should be easy to implement if we totally get rid of swig for generating the Python API. If someone would volunteer to go ahead with such an implementation, I would suggest to start with a minimal proof of concept implementation supporting only one type of sensor and one type of actuator. I would be happy to review it in a PR and if it looks good we could generalize it to the complete API. Ultimately, we could drop the current Python API and replace it with this new one. |
My current way of using supervisor controllers in Python works much like you suggest with getters and setters to let me use simple ROOT.PUDDLE.appearance.textureTransform.translation = newpos My "proxy scene tree" currently implements this using container objects (called proxy nodes and proxy fields) that wrap around Webots own python versions of nodes and fields, and for the most part simply redirect attempted references and method calls to the corresponding Webots fields and methods. I've tested this quite a lot in the labs for my class, and it works smoothly, and is a tremendous quality-of-life improvement over vanilla Webots supervisor control. Two alternative approaches (even more like what you're suggesting) would have been (2) to subclass the Webots python objects themselves to produce objects that simultaneously are instances of the Webots classes and have this sort of "pythonic" functionality, or (3) to not even bother with subclassing and rewrite controller.py to make the only version of the Webots python objects be ones with this sort of pythonic functionality. It would be fairly straightforward to adapt what I already have to produce either of these approaches. Three of the main advantages of (1) my current container-based approach are (a) that it automatically adapts to swig-produced Webots updates typically with no need for manual maintenance, (b) it minimizes the risk of there being some name-collision between the few attributes and methods I use for handling the proxies and any attributes or methods that Webots might someday give to its objects, and (c) it is entirely optional whether to use it (which is sort of good, sort of bad). Some drawbacks of (1) this generic-containers approach are (d) since I dynamically redirect references to fields and methods, those aren't known in advance so don't currently serve the linting goals that were the original topic of this thread, and (e) this runs a bit slower than an approach that cuts out my proxy middlemen could be. Approaches (2) or (3) could help with each of these, though at the cost of (a-c) above, especially increased maintenance costs for future Webots versions (or upfront costs getting swig or a post-swig script to automatically make these changes). So, anyway, you had suggested doing a "minimal proof of concept" with a single sensor and actuator. The proof of concept that I have currently instead is far from minimal, being a generic one-size-fits-all wrapper for all scene tree nodes and fields, but it does demonstrate that these can indeed be made to work in a much more "pythonic" fashion than they do in vanilla webots, and I can attest that this was a tremendous improvement in my quality of life writing supervisor controllers. (It may also be worth emphasizing that this works just with the various node and field objects that supervisor controllers use, not with the device objects that ordinary controllers use, though similar moves could be made with them.) |
That's a very nice contribution, but it looks like a helper for the supervisor API, not a replacement of the python controller library. My idea was to replace the swig-generated python controller library by a native python controller library in order to have more flexibility to introduce type annotations and provide a more "pythonic" interface. Your contribution would actually fit very well into this new python library. |
Making controller.py be manually maintained would indeed open the door to a number of useful things, including (1) type-hinting for python linters, (2) adding helpful python doc-strings, and (3) providing more "pythonic" access to some attributes and methods. (I've developed a handful of other useful "pythonic" changes I'd probably suggest including under (3) too, if we do this.) In my eyes, the biggest drawback of making controller.py be manually updated is that it would add to the workflow of changing core Webots features that require corresponding changes to controller.py. E.g., last year there were changes to how a supervisor gets contact points that led to corresponding changes in controller.py, which I think were probably done automatically by swig? There must be some advantages to having swig automatically percolate such changes to Python without requiring that the person who figured out how to make these changes in C need to know how to do this in Python, and remember to do it. It's above my paygrade to determine whether the long-term advantages of switching to manual maintenance of controller.py are worth the costs. Another option I would probably consider in your position is creating a python script that would take the swig auto-generated version of controller.py and automatically rewrite it to include things like (1-3) above (or perhaps there is a way to force swig to do everything we'd want, not sure). If this script is added to Webots' automated build process, then this would automatically percolate many Webots changes to controller.py via swig and this script, rather than requiring any sort of manual maintenance. Depending on what the Webots changes are, there might be some sorts of manual maintenance that would be good to do, e.g., adding any type-hinting or documentation that the script can't figure out on its own. But, since type-hinting and documentation are optional in Python, if no one remembers to make those changes, that would just be a minor inconvenience and wouldn't stop new Webots features from working in Python. Of course there are probably greater set-up costs to get this script going in the first place, though once you know what you're doing it's not all that much harder to script a sequence of find-and-replace steps than it is to perform them manually once. And there could be some future changes to Webots that this script wouldn't be prepared for that might require some sort of manual response, but those would likely be rare, much more rare than all the Webots changes that would require manual revisions to controller.py if we followed your proposal to stop using swig entirely. |
I believe the maintenance overhead for a new python library without swig (let's call it native) is very small. We do not add API functions everyday and whenever we add one, we need to test it in python to ensure the swig generated interface works which is not always the case and sometimes needs to be adapted in the controller.i file. This file, as you can see, requires anyhow some maintenance and is pretty complex to maintain (you need some good swig expertise and you may need to update it when swig gets an update, etc.). So my feeling is that the maintenance overhead is a little price to pay to have a better and simpler Python API and I am happy to move into this direction. |
I think there is a swig option to have it generate auto-documentation, which is often flawed (e.g., using C++ names for types rather than their Python names) but still could be a helpful starting point for manually setting up type-hinting. So I guess a useful first step would be for someone to use swig to generate a version of controller.py with that auto-documentation included. I haven't figured out how to use swig, so I'm not ideally situated to do that. It may also be useful to look at the stubs that @PeterJCLaw created that included some Python type-hinting already -- slightly out of date now, but probably straightforward to merge. Adding the type-hinting would probably be quite straightforward, if a bit tedious. I think mironix expressed some willingness to take that on. I'll also look through controller.py and think about what I would recommend regarding making things more "pythonic", including figuring out a good way to incorporate my proxy scene tree stuff. I think I will probably recommend the option I had labeled as (3) above: directly building this stuff into the controller.py, rather than adding extra subclasses or container classes. |
Ironically enough, last year I pointed out that specific flavors of This may weigh somewhat in the direction of un-deprecating the specific flavors like m = getMotor("left motor") # currently deprecated, but could easily be made to type-hint that the returned item will be a Motor
m = getDevice("left motor") # currently equivalent to the preceding, but can't automatically type hint Motor
m = getDevice("left motor") # type: Motor # Serves the same type-hinting purposes as the first would have Relatedly, though, I very often end up pairing ls = getDevice("light sensor", interval = robot.timestep ) # type: LightSensor # finds and enables this light sensor I can't remember, off the top of my head, how much variety there is in |
Your type hinting suggestion sounds very good to me. |
This comment has been minimized.
This comment has been minimized.
I'd also encourage having separate per-device-type methods, or at least supporting that as part of the API. Our project currently makes use of its own wrapper to essentially achieve that: https://github.com/srobo/competition-simulator/blob/a49f5c3fa51ee35458b4194d5d0fae4810ddeb14/modules/sr/robot/utils.py#L30-L57 (the function signature is the interesting part, not the cross-compatibility part). This approach works, but isn't especially nice. As you noted, having true per-device-type methods opens up the API to doing other things with the argument which can then be customised per the type of device. |
Another thing I've been thinking about folding into a new-and-improved controller.py is to include a Vector class that I created to use as the default format for various Webots functions that return vector-like results. This class provides python operator overloading, so e.g., you can use These Vectors are very similar to Numpy Arrays in how they do broadcasting and vectorized operations. (Numpy is a widely used Python library for doing highly efficient processing for large numerical arrays.) A related issue that I'm unsure about is how much Numpy-dependence to build into our new-and-improved controller.py. For some devices, like cameras, providing a read method that directly returns a Numpy array would be extremely helpful and vital for dealing with these efficiently in Python. One possible approach here would be to demand Numpy and just use Numpy arrays rather than my Vectors. But not everyone will have Numpy installed (and even if they do have Numpy installed, Python imports in Webots are sometimes hard to get working right), so I'm reluctant to outright require Numpy. For that reason, I implemented my Vectors in a way that makes them be interoperable with Numpy arrays, but doesn't require that Numpy be installed. (Unlike Numpy my implementation doesn't use fast C code under the hood, but inner-loop speed isn't so relevant when the vectors/arrays in question are small, as most commonly used Webots vectors are: mostly 3D translations/colors or 4D rotations.) Still, I think it would be a good idea to have at least a few methods, especially for high-bandwidth devices like Cameras, that will shunt data directly into Numpy arrays for efficient processing if Numpy is installed, and will generate an explanatory error if this method is called without Numpy installed. E.g. we could add a new camera method like the following: a = cam.getNumpyArray() |
(Incidentally, I implement Vectors as a subclass of Python Lists, so changing any part of the API that previously returned Lists to instead return Vectors should be fully backwards-compatible, since Vectors are Lists and support all the standard python List methods.) |
Before embarking on this, I should probably try to get clear on something else you said above, suggesting that we might want to use the C API instead of the C++ API that swig had been using. I had been thinking of starting with a current working version of controller.py and changing it in ways that preserve all of the underlying functionality, while adding various bells and whistles like type-hinting, documentation strings, new methods, and handling of additional/optional arguments. I think this approach would most naturally end up still using whatever controller.py currently uses, which I think is the C++ API. What would be the advantage of using the C API instead? How hard is it to translate between these? |
The advantages of using the C API instead of the C++ API are:
I see no good reason for using the C++ interface. The reason why we used it is that it was easier to handle by swig. If we drop swig, then, I see no reason to use the C++ interface for the python library. |
The main reason I see is inertia: we already have a working On the bright side, I think the tasks we're considering are quite separable: (1) adding the various bells and whistles I'd been thinking about adding like type-hinting, doc-strings, and making things a bit more "pythonic", and (2) converting all the calls to the C++ API into calls to the C API. So it seems like I'd probably be fine focusing on (1) for now, with the thought that one of the benefits will be that this will open the door to someone eventually doing (2) as well? Is it possible to gain even more separability by making the API-switch gradually, going through a period where it depends on both API's and performs some tasks via one API and some tasks via the other? Or are we forced to choose a single API and have to change everything at once? I'd feel a lot more confident changing the simple things that I know how to change, and flagging the things that I don't for someone more knowledgeable than me to maybe change later. A related question is whether there's some sort of test suite available that can check whether things get broken in this changeover process? If we're manually rewriting pretty much every API call, we'd probably want some pretty thorough testing before making the changes official. (I guess the relevant test suite would probably be a world with one or more robots containing the full panoply of devices, together with one or more Python controllers that try to interact with these devices in all the ways the API allows, and ideally a supervisor controller that can confirm whether each of these interactions produces the expected results. Of course we can't expect to test everything, but this could still do a lot to catch many of the likeliest bugs.) |
Another relevant question is what version(s) of Python we'll be aiming to support? There's a fairly deep division in the Python community between people who use a quite new version of Python 3, and old sticks in the mud who have stuck with Python 2 for 13+ years since Python 3 wasn't entirely backwards-compatible with Python 2. I believe that Webots has been using swig to create separate versions of controller.py for several recent versions of Python3 (3.7, 3.8, and 3.9), and for an ancient version of Python 2. Python 3 and Python 2 have significant incompatibilities that would make it quite challenging to manually maintain versions of controller.py for each of them (and it would also be somewhat challenging to manually keep track of the slight differences between different recent versions of 3). If we're doing this manually, I think it would be best to pick one version and stick with it, perhaps occasionally incrementing the Python version requirement if we want to make use of new features. Type-hinting in Python didn't become a thing until around version 3.5 (circa 2015), and it has received notable improvements in more recent releases, including in version 3.10 that was just released this month. The "pythonic improvements" that I'd been hoping to include likely also rely on having a somewhat recent version of Python 3, and I'm not sure the extent to which they'd even be possible in Python 2. Even if they are possible, the conversion would probably require more work than I'd want to put in. So, anyway, the fewer versions of Python you support, the greater the risk of losing some people who are unwilling to install and work with a newer version. But the more versions you want to cater to, the greater the challenge would be of manually updating versions of controller.py to go with each of them. In particular, my own willingness to contribute probably wouldn't extend much further than making it work with some specific recent version of Python3, e.g. 3.8 or even 3.10. |
Looking at the public docs, it seems only Python 3.6+ is supported? So Python 2 and 3.5 aren't really concerns any more. While there definitely are new features in more recent versions (deferred annotation processing being the main one), I'm not aware of any big changes that would be necessary to support specifically that would cause issues for supporting 3.6+. Lazy annotations can even be enabled via |
The code I'm aiming to integrate does currently use some features from 3.8+, but it should be fairly straightforward to rewrite it to work with 3.7 or even 3.6. The main lingering issue is Python 2, which still does have its own version of controller.py in Webots [edit: actually I apparently misremembered this, as it doesn't seem to be there now -- sorry], even if it's not officially supported. Python 2 has been officially sunsetted since just before the pandemic -- i.e., it no longer gets any security updates or anything -- and it seems finally to be in the process of dying, though it seems possible that some Webots users are still using it. Are we fine with abandoning it? |
Sure, we can fully drop Python 2 support. |
In case it got lost in the ensuing discussion of python versions, I'll repeat a couple of my questions from earlier:
And I'll add a new question. How would I even go about switching to the C-API? As far as I can tell, controller.py imports a python extension called |
I believe this new python API should be implemented from scratch as a new python module. Basically, it should simply load the libController shared library and call it's functions wherever needed. Dead simple. |
Did you use python ctypes to load the dll and call the API functions? Yes, a couple of functions will have to be re-implemented in Python, but that will be easy and can be taken from the previous code. |
Yes, |
Yes, that should be easy to handle by introducing specific cases for Linux/macOS/Windows. For now, you can go ahead and consider that we will take care of this later. |
Here is a very simple skeleton that works on Windows: webots.py module (to be stored in the same folder as the controller for testing): import ctypes
import os
class Robot:
controller = None
def __init__(self):
if Robot.controller is not None:
print('Error: only one Robot instance can be created per controller process.')
return
self.WEBOTS_HOME = os.environ['WEBOTS_HOME']
Robot.controller = ctypes.cdll.LoadLibrary(os.path.join(self.WEBOTS_HOME, 'lib', 'controller', 'Controller.dll'))
Robot.controller.wb_robot_init()
def step(self, time_step: int) -> int:
return Robot.controller.wb_robot_step(time_step)
class DistanceSensor:
def __init__(self, name: str):
self.id = Robot.controller.wb_robot_get_device(str.encode(name))
Robot.controller.wb_distance_sensor_get_value.restype = ctypes.c_double
def enable(self, time_step: int):
Robot.controller.wb_distance_sensor_enable(self.id, time_step)
@property
def value(self) -> float:
return Robot.controller.wb_distance_sensor_get_value(self.id)
class Motor:
def __init__(self, name: str):
self.id = Robot.controller.wb_robot_get_device(str.encode(name))
Robot.controller.wb_motor_get_target_position.restype = ctypes.c_double
Robot.controller.wb_motor_get_velocity.restype = ctypes.c_double
@property
def position(self) -> float:
return Robot.controller.wb_motor_get_target_position(self.id)
@position.setter
def position(self, p: float):
Robot.controller.wb_motor_set_position(self.id, ctypes.c_double(p))
@property
def velocity(self) -> float:
return Robot.controller.wb_motor_get_velocity(self.id)
@velocity.setter
def velocity(self, v: float):
Robot.controller.wb_motor_set_velocity(self.id, ctypes.c_double(v)) mybot_simple.py (to be used as a replacement of webots/projects/samples/mybot/controllers/mybot_simple/mybot_simple.exe): from webots import Robot, DistanceSensor, Motor
robot = Robot()
ds0 = DistanceSensor('ds0')
ds1 = DistanceSensor('ds1')
ds0.enable(64)
ds1.enable(64)
left_motor = Motor('left wheel motor')
right_motor = Motor('right wheel motor')
left_motor.position = float('inf')
right_motor.position = float('inf')
left_motor.velocity = 0.0
right_motor.velocity = 0.0
while (robot.step(64) != -1):
if ds1.value > 500:
if ds0.value > 500:
left_speed = -6
right_speed = -3
else:
left_speed = -ds1.value / 100
right_speed = (ds0.value / 100) + 0.5
elif ds0.value > 500:
left_speed = (ds1.value / 100) + 0.5
right_speed = -ds0.value / 100
else:
left_speed = 6
right_speed = 6
left_motor.velocity = left_speed
right_motor.velocity = right_speed Of course, this is just a beginning and should be improved, but basically that's a starting point. |
Yes, your skeleton pretty closely matches what I currently have, so good to see we're on pretty much the same page. I'm having trouble finding the constants in At this point, I haven't figured out a good way of determining what's in the .dll aside from just requesting various things and seeing whether they successfully return something or cause an error. The .dll itself is some sort of compressed unreadable mess, and the usual python methods for introspecting its contents don't work on this. Is there some place that I can find the source code from which that .dll was created? |
In case anyone's interested in some statistics: |
Sure, it's here. About constants, they are not defined in the DLL file, but in the include files, located here. I believe the best would be to have a python script parsing these include files to generate the equivalent constants in python (rather than maintaining this manually which would be error prone). |
About node constants, we may also decide that the new Python API will use node names instead of enums (int), that is |
The goal seems to be to have Controller.dll contain everything that any language (like Python) that can link dll's would need to thereby be able to write Webots controllers following the webots docs. If that's the goal, then it seems like this .dll should include the documented WB constants (much as For now, since I can get the constants most straightforwardly from I'm not sure how many constants correspond to Python classes that users will already be employing. A few do, like some node types available to supervisors will correspond to device types available to ordinary controllers. I can see some advantage to asking |
I wouldn't use class names as a replacement for constants. As you mentioned, it might be quite confusing and wouldn't work for all constants. But I like you idea of defining these constants in the DLL rather than in include files. I will think on what the best way to implement it. |
How much assurance is Webots offering that constants will retain their values from Webots version to Webots version? If it is assured that constants will retain their values, then (1) that makes it easier to create a one-stop repository for constant values, and (2) there may be some advantage to leaving them as integers since integers could be used in some mathematical ways, e.g. as indices of arrays, or in boolean evaluations. If, on the other hand, there's no assurance that constants will retain their values from webots version to version (e.g., if a new item might get inserted in the middle of an enumerated list, and if the expectation is that these constants will be useful only for identity comparisons) then (1) we'd probably want to automate, as much as possible, tracing back of constant names to their current values to avoid the risk of failing to keep up with a change, and (2) we'd be more free to assign new values, like informative strings, to these constants in some languages. |
Unfortunately, there is no guarantee that constants will retain their value from version to version, as we may (as we did in the past) engage with some refactoring that requires that we insert a constant at some point in between others. I just proposed a nice way to share constant values from the C libraries to the Python library so that we don't have to hard-code and maintain them at two different places. This was implemented only for the Motor constants, but should be scalable to other objects. Please feel free to continue the discussion on #3801 and to contribute to it as well by pushing improvements. |
@omichel Is this issue still relevant now that the Python API has been implemented? |
Type annotations were introduced in the new Python interface. However, they are not yet used in the Python API documentation, which would be nice. So, let's keep this issue open for now. |
Closing as duplicate of #2959. |
Aha, nice. Is there an example of using the types to check client code? Are the types published on PyPI or just as part of the main webots install? (Apologies if I've missed some docs on this) |
Those are just type annotations in the source code of the python module. It is part of the main Webots install and not published on PyPI. |
Would you be open to publishing stubs on PyPI? I'm not actually sure how to do it, but I'm imagining a tool extracting the stubs from the source and then building a package around them. That package could then be used in CI etc. where a full install would be quite heavyweight. |
Yes, that is a good idea. However, since the Python API depends on the shared library binaries of the libController, it might be tricky to set it up properly on PyPI and support all architectures (Windows, Linux and macOS). But if you are willing to try this out, feel free to explore it and let us know your findings. Eventually, we would be happy to review your PR introducing such a contribution. |
Have done some experimenting; the results are in #5472 though mostly as a PR seemed a useful way to share the results. |
I'm not sure how the Python API is built, however I was wondering whether it would be possible to create type annotations for it. This would help document the APIs as well as allow users to check that their code is using the API correctly (just as, for example, the Java compiler is able to do for users of the Java API).
At the moment I've got a set of local stubs in my project which I've manually created based on the documented API, with the types captured from a mixture of observed data types and the docs for other languages (mostly Java).
It feels like type annotations could usefully be added to the signature snippets in the docs as well as potentially being published as a stubs package on PyPI for use in type checking and as editor completions.
I'm definitely hoping that the signatures would be generated from the true source, however I'd be happy to contribute the stubs I've created as a starting point if they need to be manually defined.
The text was updated successfully, but these errors were encountered: