cyberbotics / webots

Webots Robot Simulator
https://cyberbotics.com
Apache License 2.0
3.26k stars 1.7k forks source link

Type annotations for Python API #2385

Closed PeterJCLaw closed 1 year ago

PeterJCLaw commented 4 years ago

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.

omichel commented 4 years ago

We use swig to generate the Python API from the C++ API and there is no easy way to introduce type annotations there, see https://github.com/swig/swig/issues/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 Feel free to propose a solution by submitting a new pull request in this repository. We will be happy to review it.

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.

Justin-Fisher commented 3 years ago

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.

omichel commented 3 years ago

All what you propose should be easy to implement if we totally get rid of swig for generating the Python API. I believe more and more that a totally new Python API could be created without swig and maintained manually, which should be easy because it will be very straightforward. This API could rely on the C API (instead of the C++ one) thus, removing the dependency on libCppController and other C++ dependencies. It would call the C API function directly from python. Such a library could easily include python hints and documentation with hyperlinks as described above. In addition to all these benefits, such a library should be designed to be more "pythonic" with python .value getters and setters instead of .getValue()/.setValue() methods inherited from C++.

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.

Justin-Fisher commented 3 years ago

My current way of using supervisor controllers in Python works much like you suggest with getters and setters to let me use simple .attribute notation to get/set the fields of scene tree nodes, and also to retrieve descendant nodes. E.g., grabbing a quick example, I use the following one-liner to do something that in vanilla webots would take half a dozen lines to find the the node with DEF-name PUDDLE, and then delve down through fields to find the relevant subfield and set it to a new value:

    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.)

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

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.

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

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.

Justin-Fisher commented 3 years ago

Ironically enough, last year I pointed out that specific flavors of getDevice functions like getMotor weren't really needed, when there was already a one-size-fits-all getDevice function that yielded exactly the same effects. In response, you deprecated the specific flavors and encouraged folks to switch to getDevice. However... now that we're thinking about trying to facilitate type-hinting, one-size-fits-all constructors like getDevice actually aren't very friendly for type-hinting, since it's unpredictable which flavor of device will be returned, so linters won't have any way of knowing that getDevice("left motor") will return a motor device rather than some other device, and won't automatically be able to recommend methods or documentation that are specific to Motors.

This may weigh somewhat in the direction of un-deprecating the specific flavors like getMotor, now that they actually can serve a useful type-hinting purpose separate from the generic getDevice. Alternatively, we could just make the sample Python code include explicit type hints that would accomplish the same linting purposes. This alternative is attractive because it allows novices to get away with remembering a single getDevice command rather than needing to learn a bunch of specific flavors, while still giving folks who want type hinting an option that isn't much harder than looking up a specific flavor of getFoo command.

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 getDevice with device.enable commands, so, if we're thinking of making things more "pythonic", I would probably suggest letting getFoo commands accept an optional argument to simultaneously enable them with a specified time interval.

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 device.enable calls for different Webots sensors, but the more diversity there is, the more this might weigh in favor of going back to having specific flavors of getFoo calls with their own enable options, rather than trying to do everything with a generic getDevice.

omichel commented 3 years ago

Your type hinting suggestion sounds very good to me. About adding the interval optional argument, this is also a very good idea. However, it may be a problem with some devices, such as Emitter, Pen, Skin or LED, which don't have a enable method, but I guess in this case, the library should simply issue a warning in the console.

PeterJCLaw commented 3 years ago

(sorry, misclick)

PeterJCLaw commented 3 years ago

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.

Justin-Fisher commented 3 years ago

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 (v1+v2)/2 to average vectors v1 and v2 together, and it provides numerous other conveniences like v1.x, v1.mag and v1.unit_vector to return the x-component, magnitude, and unit-vector of v1, respectively. My proxy scene tree already automatically coerces any vector-like fields that a supervisor reads from the scene tree to Vectors, which is very convenient. It would be useful to make ordinary sensors that return vector-like values do this as well.

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()
Justin-Fisher commented 3 years ago

(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.)

Justin-Fisher commented 3 years ago

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?

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

The main reason I see is inertia: we already have a working controller.py using the C++ API, and it would take effort to change which API it uses. That effort might well be worth it, for the reasons you list. But I don't know enough about the two API's to know how to change it, nor how much effort that would involve. Unless the relevant changes are straightforward and obvious, I suspect that it'd be better to have someone else make those changes who knows the C and C++ API's better than I do.

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.)

Justin-Fisher commented 3 years ago

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.

PeterJCLaw commented 3 years ago

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. (edited: 3.6 is supported on older Ubuntu)

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 __future__ import in 3.7+, but IMO are nice to have rather than being particularly arduous to work without.

Justin-Fisher commented 3 years ago

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?

omichel commented 3 years ago

Sure, we can fully drop Python 2 support.

Justin-Fisher commented 3 years ago

In case it got lost in the ensuing discussion of python versions, I'll repeat a couple of my questions from earlier:

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?

A related question is whether there's some sort of test suite available that can check whether things get broken in this changeover process?

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 _controller.pyd that presumably contains a version of the C++ API converted (perhaps by swig?) to be make its functions be executable by Python. So I guess a key step would probably be generating a similar file, except based upon the C API. Is such a file available somewhere?

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

Sorry if I'm being slow/dense on this. Are you talking about libController.a? Do you know how to import that in Python? And how to access the relevant functions in Python? Are the available functions exactly the ones that Webots docs list for the C language? (I may be wrong, but it seems like the current controller.py calls some API functions that I didn't think were documented. Maybe they were constructed by swig?)

My very strong inclination would be to start with something that works, like controller.py and at least use it as a checklist of things that should probably somehow have analogs in the new module (aside from some pretty-clearly-useless bloat that swig added).

I actually did embark on this today, and have done a fair amount of work streamlining the controller.py device creation process, in a way that should make it pretty easy to switch API's, I think, though I still haven't wrapped my head around what exactly will be different about the C-API. For now, I'm still having it use the swig/C++ API, since I know well enough how to use it and can test that my changes haven't completely broken things. So far so good...

omichel commented 3 years ago

I believe developing a python module that loads a C shared library and calls its functions is something pretty standard. Yes, you need to load libController which is named with a .dll extension on Windows, .so on Linux and .dylib on macOS, but I guess there is a generic way to achieve this. Then, yes, you should call the C API functions which are documented in the reference manual. If you cannot manage to do it, I could create a working skeleton next week into which it should be easy to port your code.

Justin-Fisher commented 3 years ago

My windows installation has libController.a and controller.dll, but not libController.dll (at least not in the controller folder).

For now I think I'll stick with the swig/C++ API, though I've been setting things up so that it should hopefully be pretty easy to switch to the C API, since there seems to be a pretty straightforward mapping between function names in the two API's, and I've streamlined most of the swig-specific stuff into just a few places where it'll be easy to turn off. My main worry is about ways that the API's significantly diverge, and I don't know them well enough to know how many of those there are.

If you do get the chance to throw together a skeleton showing how to use the C API in Python, you'll probably be quicker at that than I would, since I've never done that before, and then me making the switch to it should probably be pretty simple.

omichel commented 3 years ago

Sorry, I meant controller.dll indeed. I will give the C implementation a go next week and keep you posted.

Justin-Fisher commented 3 years ago

I don't suppose you know already what type the "WbDeviceTag" returned by wb_robot_get_device will have when it gets passed from the C API to python? Just an integer?

omichel commented 3 years ago

I believe it is an opaque C pointer, but can be casted to an integer anyhow.

Justin-Fisher commented 3 years ago

The old controller.py includes some functions that return device tags, e.g. Brake.getMotorTag, but these generally don't seem to be documented in Webots docs for the Python API. Is it fine to abandon these, or should they be kept for backwards compatibility? (I think for almost all purposes, people would be better off using documented alternatives like Brake.getMotor which returns a python Motor object, rather than a tag, and we probably want to set things up so that ordinary users never deal with tags directly.)

Justin-Fisher commented 3 years ago

Similarly, the old controller.py includes many duplicate functions that are defined both as documented static methods of classes, e.g. GPS.convertToDegreesMinutesSeconds and as undocumented stand-alone functions like GPS_convertToDegreesMinutesSeconds. This amounts to having two different names for the very same thing. In general, the documented version would probably be preferred programming practice. Is it fine to abandon the undocumented versions, or should they be kept for backwards compatibility?

Justin-Fisher commented 3 years ago

Incidentally, I've gotten Controller.dll to load in Python, so I don't need you to set up a skeleton for doing that. However, I've only done this in Windows, and my understanding is that the right way to do this will vary from OS to OS, including the names of the file that we'll need to import. So at some point someone will probably need to figure out how to generalize this for other OS's.

It looks like there are a fair number of functions for the Python and C++ API's that don't have analogs in the C API. If we're aiming to preserve backwards compatibility, and our shift to the C-API will leave Python without access to their C++ implementations, then I guess someone will have to re-implement them in Python?

omichel commented 3 years ago

Did you use python ctypes to load the dll and call the API functions? If so, it should be pretty straightforward to port it to Linux and macOS.

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.

Justin-Fisher commented 3 years ago

Yes, ctypes.CDLL gives you something that's analogous to the _controller that the old controller.py imported. I think the main thing that will vary is just the name of whatever Controller file I'm supposed to load (e.g. Controller.dll on Windows). Do you know what that would be for each Webots operating system? And will that file always be located one directory above the Python directory in all operating systems?

omichel commented 3 years ago

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.

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

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 Controller.dll. E.g., the docs say that there is a constant called WB_NODE_CONE, but the .dll doesn't seem to contain this, whereas the swig-produced _controller.pyd did have a constant for this which it called Node_CONE.

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?

Justin-Fisher commented 3 years ago

In case anyone's interested in some statistics: 861 API-calls beginning _controller. occur in the old controller.py 846 unique items remain after we remove 15 duplicates. 698 of these are important items, whereas 148 were swig artifacts that we don't really need in the C-API, 167 of the important items are constants, which, as I mentioned above, I can't yet find in the C-API. 388 of the important items are functions with easily-findable analogs in the C-API (via systematic name transformations) - yay! 143 of the important items are functions that unfortunately lack any easily-findable analog in the C-API, either because they were named in idiosyncratic ways that evaded my systematic transformations, or because there simply are no analogs and these will need to be rewritten. These cluster together, with especially large clusters for (mostly) high-bandwidth devices like Cameras, Lidar, Contact Points, Radar, and Mouse.

omichel commented 3 years ago

Is there some place that I can find the source code from which that .dll was created?

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).

omichel commented 3 years ago

About node constants, we may also decide that the new Python API will use node names instead of enums (int), that is 'Cone' instead of WB_NODE_CONE. To achieve this, we could add a new function named wb_node_get_enum(const char *name) in node.c that will return the node enum as an int when passed the node name (the reverse function of wb_node_get_name(WbNodeType t))

Justin-Fisher commented 3 years ago

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 _controller.pyd already does), rather than forcing languages to parse other files to reconstruct them.

For now, since I can get the constants most straightforwardly from _controller.pyd, I guess I'll do that.

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 if node.type==Motor. But I'm not sure whether it would be helpful or confusing to use the corresponding device class as the type-constant for a scene-tree node, since supervisors can't actually use scene tree nodes as devices (though ultimately it would be nice if they could!). I think most constants won't automatically have corresponding Python types, so there may not be all that much mileage to be gained by a patchwork system that rewrites a few of them to Python types.

omichel commented 3 years ago

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.

Justin-Fisher commented 3 years ago

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.

omichel commented 3 years ago

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.

ygoumaz commented 1 year ago

@omichel Is this issue still relevant now that the Python API has been implemented?

omichel commented 1 year ago

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.

ygoumaz commented 1 year ago

Closing as duplicate of #2959.

PeterJCLaw commented 1 year ago

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)

omichel commented 1 year ago

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.

PeterJCLaw commented 1 year ago

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. (If that's interesting I'd be happy to open a separate issue for it)