Skip to content
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

qtile cmd-obj: convert integer arguments into ints #2435

Closed
wants to merge 1 commit into from

Conversation

m-col
Copy link
Member

@m-col m-col commented May 1, 2021

This checks through the argument list as passed from the shell when
using qtile cmd-obj ... -a <arguments> and converts any arguments that
appear to be integers into ints. This makes the commands that expect
ints receive ints, rather than strs. Fixes #2433.

This checks through the argument list as passed from the shell when
using `qtile cmd-obj ... -a <arguments>` and converts any arguments that
appear to be integers into `int`s. This makes the commands that expect
`int`s receive `int`s, rather than `str`s. Fixes qtile#2433.
@ramnes
Copy link
Member

ramnes commented May 1, 2021

I'd rather inspect type hints. This will probably break functions taking a string as argument and receiving a legit string made of digits. Groups, for example, are often called 1, 2, 3...

@m-col
Copy link
Member Author

m-col commented May 1, 2021 via email

@elParaguayo
Copy link
Member

Oh yeah good point. How can type hints be accessed?

Well dbus-next inspects type hints as it uses custom hints to match dbus signatures. I think it uses the inspect module.

There's this: https://docs.python.org/3/library/inspect.html#introspecting-callables-with-the-signature-object

@m-col
Copy link
Member Author

m-col commented May 2, 2021

There's this: https://docs.python.org/3/library/inspect.html#introspecting-callables-with-the-signature-object

Great, thanks for this, looks useful

@m-col
Copy link
Member Author

m-col commented May 2, 2021

The inspect stuff looks pretty straightforward. Where in the whole IPC communication makes the most sense to cast the arguments? Ideally it would be in the client side before sending the arguments to the server, and not somewhere where it is attempted during normal server-side operation (e.g. when resolving keybindings). It only would be needed when using a client from a shell too, not in Python clients. It seems that there isn't a client-side way to get a handle on the command that the client then wants to call. Looking into the IPC stuff I'm beginning to think this wasn't as simple as it seemed at first 😅

@github-actions
Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@github-actions
Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@m-col
Copy link
Member Author

m-col commented Nov 14, 2021

It makes sense for this to come after #2727 is merged

@github-actions
Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@elParaguayo
Copy link
Member

We should keep this open as it could be improved!

@elParaguayo elParaguayo reopened this Mar 14, 2022
@elParaguayo
Copy link
Member

Taking a look at this.

Could we do something like this:

  • create a method that returns the argument signature for the method (like an introspect method)
  • have cmd_obj call this first and then try type conversions based on the signature

It would mean two calls instead of one but at least we can keep it client side.

Alternatively, if we want something on the server, we need a client to flag when everything is sent as a string so the server knows to do some type conversion.

@m-col any thoughts/preference?

@github-actions
Copy link

This PR is stale because it has been open 90 days with no activity. Remove stale label or comment or this will be closed in 30 days.

@tych0
Copy link
Member

tych0 commented Mar 17, 2024

I guess we could do something for option types (i.e. optional None), but yeah, for everything else it seems like a code smell to me at least.

@tych0
Copy link
Member

tych0 commented Mar 17, 2024

I guess the other problem is for compound types, e.g. List[int] or something. But for basic types it should work, which is maybe better than nothing?

@tych0
Copy link
Member

tych0 commented Mar 17, 2024

Ok, it's a little (well, a lot) more verbose when trying to handle union types: https://github.com/tych0/qtile/tree/type-conversions

this is all still untested. let me know if this looks interesting and i'll try to make it work for real.

@fiorematteo
Copy link
Contributor

I experimented a bit with tych0@7f8aef7, and came up with this:

def lift_arg(typ: Union[type, str], arg):
    # annotations can be strings with the type name, or actual types
    if isinstance(typ, str):
        typ = eval(typ)
    if not isinstance(typ, type):
        raise TypeError("Could not evaluate to an actual type")

    # for stuff like int | None, allow either
    if get_origin(typ) is Union:
        for t in get_args(typ):
            if t == NoneType:
                # special case None? I don't know what this looks like
                # coming over IPC
                if arg == "":
                    return None
                if arg is None:
                    return None
                continue

            try:
                return lift_arg(t, arg)
            except TypeError:
                pass
        # uh oh, we couldn't lift it to anything
        raise TypeError(f"{arg} is not a {typ}")

    if get_origin(typ) is list:
        # i am explicitly too lazy to support heterogeneous lists, even
        # though it would be easy to do here. that would be an awful
        # user api.
        for t in get_args(typ):
            out = []
            try:
                for elt in arg:
                    out.append(lift_arg(t, elt))
            except TypeError:
                pass
            return out
        # uh oh, we couldn't lift it to anything
        raise TypeError(f"{arg} is not a {typ}")

From my testing this can handle builtin type as well as unions and lists.
I don't think it's a good idea to try and support more complex types.

The most annoying part is that annotations are saved as strings so we need to call eval to get the actual type object.
I'm not sure this is a good approach, handling the string case in each command seems simpler.
Also because for any of this to work we need to first annotate all commands.

@tych0
Copy link
Member

tych0 commented Mar 25, 2024

Yeah, I don't want to do eval; we can get rid of the strings if we get rid of all the from __future__ import annotation, which I believe is okay now that we only support 3.9? But maybe I'm missing something there.

@elParaguayo
Copy link
Member

I thought there was another reason we needed to use that line but can't remember what it was. I agree it would be nice to get rid of it.

Also, a few other ideas:

  1. Should/could we limit argument types for exposed commands to basic types? If they're designed to be used with an external command interface, they shouldn't use complex types in the first place.
  2. We could add a parameter to @expose_command to take a custom function used to parse more complex types
  3. Ideally, we should limit any sort of type conversion activity so it only happens when calls are made via IPC i.e. not called directly or via lazy calls.

@tych0
Copy link
Member

tych0 commented Mar 25, 2024

I believe (3) is happening with my patch tych0@7f8aef7 although I didn't update the commit message.

I wonder if we could invent a convention and have a @staticmethod that's def from_ipc(data: str) -> ...? Then we can do some kind of test in the lifting code and people still only have to define it once per-type, instead of at every call site.

@tych0
Copy link
Member

tych0 commented Mar 26, 2024

Ok, reading https://peps.python.org/pep-0563/ ; it seems like we should not drop from __future__ import annotations since that is what's going to happen in the future, i.e. delayed type evaluation, so things are strs and not their actual type. That seems... unfortunate to me.

they talk about eval() there, but then this one interface.py file has to import everything that could possibly ever be considered an arg type, which is also annoying.

tych0 added a commit to tych0/qtile that referenced this pull request Mar 26, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
@tych0 tych0 mentioned this pull request Mar 26, 2024
@tych0
Copy link
Member

tych0 commented Mar 26, 2024

Ok; here is a maybe-not-embarassing PR: #4741

tych0 added a commit to tych0/qtile that referenced this pull request Mar 30, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 30, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 30, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes #2433
Fixes #2435
Fixes #4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Mar 31, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? But that's even uglier. Anyway.

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to be imported in
interface.py, hence the new noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 1, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 1, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 7, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 8, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 8, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 8, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

This works by adding a new parameter to the IPC protocol (perhaps we should
document this somewhere? but it doesn't need to be stable IMO, since
everyone will presumably use libqtile to implement it) that's a boolean
flag. If true, qtile will attempt to invoke the type lifting code, and it
won't otherwise.

This means that users can choose whether or not they want to invoke the
type lifting code, or set it to false and use the struct marshalling as
before. Throughout the tree we have a few callers, I have hard coded them
all to false with the notable exception of the `cmd-obj` subcommand, which
is the one place I believe we (currently) want to do this.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
tych0 added a commit to tych0/qtile that referenced this pull request Apr 21, 2024
Let's use our type annotations to resolve how to lift strings into whatever
their right type is.

This works by adding a new parameter to the IPC protocol (perhaps we should
document this somewhere? but it doesn't need to be stable IMO, since
everyone will presumably use libqtile to implement it) that's a boolean
flag. If true, qtile will attempt to invoke the type lifting code, and it
won't otherwise.

This means that users can choose whether or not they want to invoke the
type lifting code, or set it to false and use the struct marshalling as
before. Throughout the tree we have a few callers, I have hard coded them
all to false with the notable exception of the `cmd-obj` subcommand, which
is the one place I believe we (currently) want to do this.

For now, this just works on "normal" base+hand handled compound types. I have
implemented stuff for Union and Literal, since we have those kinds of type
annotations. However, I have not implemented e.g. lists as a compound type,
since as near as I can tell we do not have any exposed commands that take
lists (although it would not be hard, I just didn't want to add a bunch of
unused code; see the discussion on the issues cited below for some sample
code if you're looking at this in the future).

I put normal in quotes, because of e.g. the behavior with bool. It's a bit
ugly, but... best to only have to write it once. Maybe we should use eval()
for some of these instead? It would let us "parse" lists etc.
potentially...

For supporting "custom" types (i.e. ones defined in libqtile somewhere that
are more complex than just a constructor that accepts a string), we could
do some kind of poking around in the type for a `from_str`, e.g.:

    >>> from __future__ import annotations
    >>> class C:
    ...     def from_str(s: str) -> C:
    ...         pass
    ...
    >>> class D:
    ...     def foo(c: C): pass
    ...
    >>> typing.get_type_hints(D().foo)["c"]
    <class '__main__.C'>
    >>> hasattr(typing.get_type_hints(D().foo)["c"], "from_str")
    True

again, though, we don't have any of these, so no need to define this
convention right now.

Finally, one annoying part of all this is that all of the type annotations
that anyone uses in the qtile code base need to exist in interface.py,
hence the new list noqa import. Otherwise, you end up with,

    2024-03-25 19:35:19,812 libqtile loop.py:_handle_exception():L62  Exception in event loop:
    Traceback (most recent call last):
      File "/home/tycho/packages/qtile/libqtile/ipc.py", line 235, in _server_callback
        rep = self.handler(req)
      File "/home/tycho/packages/qtile/libqtile/command/interface.py", line 357, in call
        params = typing.get_type_hints(cmd, globalns=globals())
      File "/usr/lib/python3.10/typing.py", line 1871, in get_type_hints
        value = _eval_type(value, globalns, localns)
      File "/usr/lib/python3.10/typing.py", line 327, in _eval_type
        return t._evaluate(globalns, localns, recursive_guard)
      File "/usr/lib/python3.10/typing.py", line 694, in _evaluate
        eval(self.__forward_code__, globalns, localns),
      File "<string>", line 1, in <module>
    NameError: name 'ColorType' is not defined

this is annoying, but IMO a small price to pay for only having to write
this code once. If we end up with circular dependencies or such, we can
split this code out into its own module, but for now it seemed to work
fine.

This is also the reason for the `Layout` and `_Extension` classes in that
file: they act as sentinels for the real types, since we can't really
construct those. Right now this won't work if someone tries to pass us a
real Layout, but we could use eval() to make it work if we wanted to.

Fixes qtile#2433
Fixes qtile#2435
Fixes qtile#4737

Signed-off-by: Tycho Andersen <tycho@tycho.pizza>
@tych0 tych0 closed this in 43755a9 Apr 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Development

Successfully merging this pull request may close these issues.

Error using qtile cmd-obj with an argument meant to be an int
6 participants