Support automatic XML introspection generation #29

xZise · 2016-10-18T18:19:06Z

This enables a class to generate automatic XML introspection data. This is probably not the final version, especially as I noticed that you use tab indentation while I use spaces (the TypeError exception for example is copied and thus uses tabs).

But the main reason for already open this request now is to determine if this is actually needed or wanted.

I unfortunately haven't actually tested this on an actual D-Bus service as I don't actually have written one yet. I'll probably test that as well when there is interest.

LEW21 · 2016-10-19T17:33:54Z

It's hard to say if it's a good idea or not.

In most cases, the DBus Introspection XML is a part of a pre-existing standard (like MPRIS), and the programmer can simply copy the XML file without needing to worry how to exactly replicate its behavior in his Python code. I guess we could also provide a object validator that automatically checks if all required methods are implemented, and that they take correct number of arguments; or even a XML to Python skeleton converter for quick starting implementation.

However, when there is no pre-existing standard, and the programmer is creating a new one, it can be quite easier to write everything in Python, and automatically generate XML data - as XML is not really a nice format to write by hand.

Also, having the argument data types specified in the Python source code might be useful for its readability. In fact, we could automatically test if the parameters have correct types even when methods are not called through DBus, but from other Python code - which might be useful for autotests, and for objects that are both published on DBus and used internally by the application. This suggests that type-related decorators could be implemented as a generic, non-dbus related tool, like signals in pydbus.generic.

But we don't always need type decorators. Without them, we can generate a skeleton XML file - listing all the methods and their argument names; but specyfing their arguments as "TODO" - to help programmer with writing the XML file.

I guess, we can split this feature into two smaller ones - the one about generic type-related decorators; and the one about generating DBus XML from a given Python class. And the first one should expose metadata that will let the second one automatically fill out the TODOs, and let the user publish the object without touching the XML at all. Both are useful and self-contained, and together they have synergy.

As an added bonus, developers will be able to quickly change from using autogenerated XML to using a XML file when they decide to freeze the API and publish it as a standard; without a need to remove type decorators, as they're still a useful part of the code.

LEW21 · 2016-10-19T17:34:01Z

OK, so now back to your pull request:

@Signalled is a bad idea, as not every call to the setter will change the value (and therefore should emit the signal; for example, we don't want to emit if the new value is equal to the old value); and not always the value is changed by a call to the setter (for example if we're observing some remote / hardware variable). Normal @PropertyName.setter + emitting the PropertyChanged signal manually is a better way here. (pydbus in the future might provide its own property() that's automatically handling getting, setting and emiting the signal; but that's outside of the scope of this feature)
@dbus_method/@dbus_property should be in pydbus.strong_typing module, and should be called @typed_method((arg,...), ret), @typed_property(t) and typed_signal(arg,...). You don't have to implement type checking, I can implement that later.
gen_introspection should live in pydbus.xml_generator module and should be called generate_introspection_xml(require_strong_typing=False); it should be imported into the main pydbus module. It should generate XML introspection for all members that start with an uppercase letter and are not in ("PropertiesChanged", "Get", "GetAll", "Set"). For the strongly typed ones, it should output types to the XML; for untyped ones it should use "TODO" instead
@dbus_interface should also live in pydbus.xml_generator module and be imported into main pydbus module. I'm not sure how it should be called, as I don't want people to think it's the recommended way to provide XML introspection; but on the other hand, it shouldn't be too complicated. I lean toward @autogenerate_introspection_xml(interface_name).

Note that it should work in the same way as

class xxx(object):
    ...

xxx.dbus = generate_introspection_xml(xxx, require_strong_typing = True)

It would be awesome if you could put the strong_typing-related things in one commit, and generate_introspection_xml-related things in a second one.

xZise · 2016-10-19T19:45:24Z

Okay I'll look more closely through your suggestions but thanks for your response. I have fixed the tab/spaces issue, so future commits will be fixed (I'm going to change this commit here too).

Regarding @signalled (point 1): This is up to the developer, because @dbus_property generates a normal @property so using @foobar.setter will work and you don't have to use @signalled. It is only there so that it automatically populates that XML entry.

And regarding the weakly typed methods and properties (point 3), @dbus_property and @dbus_method still provide the functionality of defining/overriding the interface used.

Also gen_introspection already makes it possible to use it like you suggested by explicitly doing clazz.dbus = gen_introspection(clazz).

LEW21 · 2016-10-19T20:05:19Z

@Signalled - right, I've forgotten about this annotation. So this is necessary, but I still don't think that it should send the signal automatically. And probably should be used on the getter, not the setter.

@dbus_property/method - I don't really understand. I think we can safely assume that all names starting from an uppercase letter are public, as normal python names start from lowercase letters, and everything magical starts from _. If somebody adds a new method starting with an uppercase letter, he probably wants to make it public; and if not, he's writing a really confusing piece of code.

About gen_introspection - yup, I know :D

xZise · 2016-10-19T21:56:18Z

But to which interface does the method or property belong? With @dbus_interface it could belong to that name, but it is possible to override that behaviour and define to which interface a method/property should belong with the @dbus_method decorator:

@dbus_method(interface='foo.bar')
def Example(self):
    pass

This doesn't mean that it must be mandatory. Via @dbus_interface it is possible to specify a default interface and it should be possible to just add all uppercase-starting-methods. What I mean is that this decorator has still a use, just to override the interface (and not to define any types).

Regarding checking the types I first was thinking whether it could infer the types from the signature:

def Example(self) -> int:
    return 42

But this is afaik not really possible, as int could be mapped to multiple types in D-Bus. Now when you mentioned a tester for this I first thought that this might have similar issues. But you only work in the other way (e.g. you check that Example returns actually an int) and that is probably more easily.

I just remember that I wanted to add “autocomplete” features (e.g. if the interface is .foobar to add org.freedesktop).

LEW21 · 2016-10-19T23:31:35Z

Objects implementing multiple interfaces are a bit of a complicated case. There are cases like MPRIS2, where all methods are grouped into 4 interfaces, and 3 of them are extensions; and there are cases where they're used for versioning, or dynamically added and removed based on some events (like a optical drive, that adds an interface when a CD is inserted).

In the MPRIS case, I guess annotating decorators are the way to go. The other two cases aren't currently supported by pydbus in any sane way, so they're out of scope here.

So, we have two ways to specify MPRIS's ActivatePlaylist method:

@dbus_method(("o"), interface="org.mpris.MediaPlayer2.Playlists")
def ActivatePlaylist(self, playlist_path):
    ...

@interface("org.mpris.MediaPlayer2.Playlists")
@typed_method(("o"))
def ActivatePlaylist(self, playlist_path):
    ...

I feel the second one is prettier, as we're stating two different facts about the method; and the "at interface" sounds semantically well.

The @interface decorator of course can be used also for properties and signals.

However, while thinking about it more, I've found another possibility:

@interface("org.mpris.MediaPlayer2.Playlists")
def ActivatePlaylist(self, playlist_path: "o"):
    ...

And this is the cleanest option. However: Python 2 does not support annotations :D But while we could provide a Python 2 fallback (2nd option, with @typed_method renamed to @annotate_method, and without type checking), this is a place where we're clearly using a Python 3 feature, and we can direct Python 2 users to the existing, XML file based method of specifying interfaces. And I feel there're already too many hacks for Python 2 in pydbus, and it's time to give people reasons to migrate to Python 3.

In the future, @TypeChecked decorator could be added to verify the annotations at runtime, but it's not really required now, and outside of scope of this feature.

Also, as it's really easy to annotate methods this way, I don't think that require_strong_typing=False is useful anymore - we can always throw an exception when a unannotated method is found.

LEW21 · 2016-10-19T23:32:57Z

(Of course for methods on the default interface, @interface is not required.)

xZise · 2016-10-21T11:32:02Z

Okay I'm currently working on implementing your suggestions. One question I have is, what do you mean with @typed_signal? Maybe that is related to your confusion regarding the usage of @signalled, but I don't think that makes sense. The XML generator basically only checks the getter to determine what type is required.

xZise · 2016-10-21T11:41:48Z

Okay I didn't noticed you had answered in the mean time, so I have actually worked on using decorators and not annotations. I personally prefer that route, as it won't break Python 2.7 (which is still widely used) and I don't think it is that much of a overhead.

As I like your second suggestion, just using one @interface decorator, and as I have already implemented the strongly typed decorators, I could try add annotation support as well to actually look how much of a overhead this would create (and I'm probably going to push it to the repo so you can take a look as well).

LEW21 · 2016-10-21T13:50:22Z

About typed_signal - objects can send signals, and currently that's being defined with

from pydbus.generic import signal

class X:
   SignalName = signal()

To autogenerate XML for them, we need a typed_signal that takes the types of signal's arguments.

xZise · 2016-10-21T14:33:45Z

Ah okay you are right. But that might be a bit more complicated as you cannot annotate assignments, so it would be something like this:

class X:
    SignalName = typed_signal(TYPES)

(having to call signal inside typed_signal seems superfluous)

I have applied (probably) all of your suggestions except using annotations, but hopefully I'll have that today. (No guarantee regarding typed_signal though, I don't know yet how to approach it)

LEW21 · 2016-10-21T14:45:22Z

That's why I haven't suggested typed_signal as a decorator :D I didn't write "@" before its name :D

Probably it should be a class that inherits pydbus.generic.signal, and has a different constructor.

xZise · 2016-10-21T14:52:49Z

Yeah I noticed that. I just wanted to make sure we are on the same page regarding it not being a decorator (especially if you consider that everything else are basically decorators).

xZise · 2016-10-21T22:52:24Z

Okay I have now uploaded the changes I did. It currently has also some support for signals but there are still a few questions remaining things. For one you used lowercase names (like signal), while the Python convention is to use CamelCase for class names (like TypedSignal). I followed the Python convention for now. It is similar with the imports which are alphabetically sorted, then by “type” (first import X and then from X import Y) lastly by “source” (first stdlib, then 3rd party and then local packages).

I also haven't added any tests for the annotations yet, as they cannot be used together with Python 2, so they need to be separate files. ~~I actually haven't set up pydbus in Python 3 because the default Python is 2.7.~~ EDIT Actually I can just call python3 😒

Also the TypedSignal cannot use annotations at all and also needs to define the name too. Currently it is all just in one parameter which is directly used in the for loop (so a list of tuples would work). Maybe there are better approaches? It doesn't actually add simple signals, but I just wanted to get this revision out first.

LEW21 · 2016-10-23T13:38:11Z

doc/tutorial.rst

@@ -265,7 +255,7 @@ in a ''dbus'' class property or in its ''docstring''. For example::
      def SomeProperty(self):
        return self._someProperty

-      @SomeProperty.setter
+      @signalled(SomeProperty)


I think this should work this way instead:

@emits_changed_signal @property def SomeProperty(self): ... @SomeProperty.setter def SomeProperty(self, value): ...

Especially as a signal might be sent even for read-only properties; and it's absolutely useless for write-only ones.

How do signals on read-only properties work? When they are changed internally?

For example, our object is a "CD drive", and our property is "DiskAvailable". User presses the physical eject button, and kernel ejects the drive. Our daemon detects that fact, and emits a signal.

LEW21 · 2016-10-23T13:54:11Z

pydbus/strong_typing.py

@@ -34,3 +35,10 @@ def decorate(func):
 		verify_arguments(func)
 		return func
 	return decorate
+
+
+class TypedSignal(signal):


I've used "signal" and not "Signal" to be consistent with "property" (which is a class), "function" and "method" (which are not exported, but can be found by type(A.method) and type(A().method)).

Signals, just like properties, look more like language features, and less like normal classes (and are language features in languages like C++/Qt). Python's CamelCase convention for class names is used mostly for domain-specific things, and builtin types (int, str, etc) and syntax-extension-wannabes (property, classmethod, etc) don't follow it.

Also, as we have a typed_method and a typed_property, TypedSignal looks quite strange.

Shouldn't it be typedsignal (like staticmethod) by that logic?

That's a good question. Following the builtins's and functools's convention, it should. While I feel with _ it's more readable, we shouldn't break the rule.

So yeah, the correct names are typedproperty, typedmethod and typedsignal.

LEW21 · 2016-10-23T13:58:18Z

pydbus/strong_typing.py

+
+class TypedSignal(signal):
+
+	def __init__(self, arguments):


I think it should be *arguments, as

SomethingHappened = typed_signal("s", "i")

is a more obvious thing to write than

SomethingHappened = typed_signal(("s", "i"))

Especially in the case of single-argument signals.

Well it actually needs to be a (name, type) tuple, so in varargs it still would be typed_signal(("arg1", "s"), ("arg2", "i")). And kwargs won't work as the order is not defined. Also I actually didn't use varargs to be in line with typed_method, which doesn't use them.

I'm not in love with typed_method's interface, but as it's only a fallback for people that can't use Python 3, I don't really care. On the other hand, typed_signal should be pretty.

Okay what would you suggest? typed_signal(("arg1", "s"), ("arg2", "i"))?

Hm... Realistic example would look like that:

PropertiesChanged = typed_signal(("interface_name", "s"), ("changed_properties", "a{sv}"), ("invalidated_properties", "as"))

Not the prettiest thing ever, but I don't think we can find anything better with this syntax.

However, this one looks awesome (and Python3-only, so the previous solution is useful as a fallback):

@signal def PropertiesChanged(interface_name: 's', changed_properties: 'a{sv}', invalidated_properties: 'as'): pass

(Note, it can be done with pydbus.generic.signal, there is no need for another class here)

(2nd note: Sorry for constantly changing my opinion :D I can add the prettier syntax myself after merging the patch if you prefer :D)

You last suggestion is fascinating! In theory you could define signals as methods (irrespective of this patch), and then you could either use the Python 3 syntax or @typed_method to specify the parameter types.

So in theory @typed_signal doesn't need to be implemented at all. I'll try to look at that feature because then it doesn't need to introduce something that isn't necessary actually.

We still need to know if this function is a method or a signal.

Well for that there is the @signal decorator. For example:

# In Python 3 @signal def PropertiesChanged(interface_name: 's', changed_properties: 'a{sv}', invalidated_properties: 'as'): pass

# In Python 2 @signal @typed_method(("s", "a{sv}", "as"), None) def PropertiesChanged(interface_name, changed_properties, invalidated_properties): pass

Now in both cases the decorator will store the method inside the signal class itself. And if the introspector comes along and sees a signal class it can access that method object and then do the almost same stuff it'd do to a normal method. The only difference is that it won't allow a return type (as far as I understand it that isn't a thing in D-Bus) and that no argument has a direction defined (or if it were it should be out not in).

As a matter of fact I have actually already implemented it that way and just finished with cleaning it up. So if you wanted I could upload it soonish. (As I write this I noticed that I haven't verified that @interface behaves as expected)

The only ugly thing there is the superfluous pass block. But I think apart from that it is actually quite nice.

Awesome.

BTW, I've initially forgotten about self, but it probably should be here too, as this isn't a static method.

LEW21 · 2016-10-23T14:09:12Z

pydbus/xml_generator.py

+		raise ValueError(
+			"Default values are not allowed for method "
+			"'{}'".format(function.__name__))
+	# TODO: Check if vargs or kwargs are actually allowed in dbus.


No, they aren't.

However, you should skip arguments with names starting from "dbus_", as "dbus_context" might be passed as a keyword argument automatically by pydbus: 78daafa

Ah okay thanks for the information (both that vargs and kwargs are not allowed and that you have dbus_context). But do I see it correctly that this dbus_context has nothing to do with vargs or kwargs? If a method has keyword arguments it won't populate dbus_context would it? (But nevertheless it should skip those arguments)

Yeah, dbus_context won't get passed if somebody is using **kwargs. But we need to handle the case when it is stated explicitely in the function's signature (like def MyMethod(self, sth1, sth2, dbus_context=None):)

LEW21 · 2016-10-23T14:15:11Z

(GitHub collapsed one of two TypedSignal-related comments as outdated, you might need to click "Show outdated")

If the `signal` class is used as a decorator it'll use the decorated function as the name and also cache the decorated method.

It enables to automatically inspect a class and generate XML introspection data by looking through each method, signal and property. As the types are not defined it won't add them to the data.

The decorators add information about the types a method or property can accept or return. The XML generator can optionally require that every method and property has the information defined. Alternatively it adds the possibility to use annotations instead, but that is a Python 3 only feature.

xZise · 2016-10-25T20:40:15Z

Regarding the self parameter in signal dummy methods: This is actually not as important as the method is (afaik) never bound to the class and even then it would never be called. So it could actually not skip defining self. Now my patch actually uses it because it can reuse the code to extract the argument types, which is automatically discarding the first argument.

AlexejStukov · 2016-11-22T15:31:08Z

Ok, I know this may be too late, but you could use the new type hints with python2.7 in the same file. All you have to do is move them to a single line comment beneath the function signature (see here). But I don't really know if there is an equivalent for inspect.signature that works on python2.7 or if you would have to write something that handles that.

xZise · 2016-11-22T17:39:08Z

This patch supports type hints using the Python 3 syntax but there is no such support for Python 2. One major reason is that it would have to actually parse the file itself, because (as far as I can tell) the annotations using comments aren't actually parsed by Python.

poncovka · 2017-06-15T13:47:54Z

Hi, how does it look with this pull request? We would like to use this approach in anaconda.

poncovka · 2020-08-10T09:57:35Z

We have implemented our own solution with type hints and we heavily depend on it in our project. This implementation is now part of the dasbus library (https://github.com/rhinstaller/dasbus) that we have started to use instead of pydbus.

xZise force-pushed the introspect branch from 45784ba to 2ff9642 Compare October 21, 2016 22:42

xZise force-pushed the introspect branch from 2ff9642 to 3a113bc Compare October 21, 2016 23:28

xZise mentioned this pull request Oct 22, 2016

Use pytest #30

Open

LEW21 requested changes Oct 23, 2016

View reviewed changes

xZise added 3 commits October 25, 2016 13:13

Support signal class to be a decorator

7adbd73

If the `signal` class is used as a decorator it'll use the decorated function as the name and also cache the decorated method.

Automatically generate XML introspection data

2816403

It enables to automatically inspect a class and generate XML introspection data by looking through each method, signal and property. As the types are not defined it won't add them to the data.

xZise force-pushed the introspect branch from 3a113bc to c8de073 Compare October 25, 2016 11:46

Support automatic XML introspection generation #29

Are you sure you want to change the base?

Support automatic XML introspection generation #29

Conversation

xZise commented Oct 18, 2016

LEW21 commented Oct 19, 2016

LEW21 commented Oct 19, 2016 • edited Loading

xZise commented Oct 19, 2016 • edited Loading

LEW21 commented Oct 19, 2016

xZise commented Oct 19, 2016

LEW21 commented Oct 19, 2016

LEW21 commented Oct 19, 2016

xZise commented Oct 21, 2016

xZise commented Oct 21, 2016

LEW21 commented Oct 21, 2016 • edited Loading

xZise commented Oct 21, 2016 • edited Loading

LEW21 commented Oct 21, 2016

xZise commented Oct 21, 2016 • edited Loading

xZise commented Oct 21, 2016 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

LEW21 commented Oct 23, 2016

xZise commented Oct 25, 2016

AlexejStukov commented Nov 22, 2016

xZise commented Nov 22, 2016

poncovka commented Jun 15, 2017

poncovka commented Aug 10, 2020

LEW21 commented Oct 19, 2016 •

edited

Loading

xZise commented Oct 19, 2016 •

edited

Loading

LEW21 commented Oct 21, 2016 •

edited

Loading

xZise commented Oct 21, 2016 •

edited

Loading

xZise commented Oct 21, 2016 •

edited

Loading

xZise commented Oct 21, 2016 •

edited

Loading