Fix/some type hints

[vvanglro]

Better type hints.

[codecov[bot]]

Codecov Report

Merging #213 (ed460f4) into main (511c8c5) will increase coverage by 0.0%. The diff coverage is 100.0%.

@@          Coverage Diff          @@
##            main    #213   +/-   ##
=====================================
  Coverage   89.8%   89.9%           
=====================================
  Files          6       6           
  Lines        732     734    +2     
  Branches     154     155    +1     
=====================================
+ Hits         658     660    +2     
  Misses        49      49           
  Partials      25      25           

Impacted Files	Coverage Δ
asyncio_mqtt/client.py	81.7% <100.0%> (+<0.1%)	:arrow_up:
tests/test_client.py	100.0% <100.0%> (ø)

[JonathanPlasse]

What is the motivation for those changes?

[frederikaalund]

What is the motivation for those changes?

Seems to be for python 3.7 support. :) I personally welcome this effort! :+1: At least until we officially drop support for 3.7. What do you guys (asyncio-mqtt authors/maintainers) think? It is a step backwards so an alternative to this PR is to drop support for old versions of python.

@vvanglo thanks for another great PR! We (the authors/maintainers) have to align on the direction first.

[vvanglro]

I think this is a general method of type hinting, whether it is python3.7 or later. And the delay detection is removed. Personally, I don't like to use delay detection very much.

[vvanglro]

There is another motivation because of this. 😄 Tips for pylance in vscode:

[empicano]

What do you guys (asyncio-mqtt authors/maintainers) think?

I'm not a good authority on typing because I don't use it that much, I'm fine with both versions 👍

Apart from that, Python 3.7 is almost at end-of-life so I think it's reasonable to drop support as well if it's extra effort for us.

[JonathanPlasse]

There is another motivation because of this. smile Tips for pylance in vscode:

It should just not be split into multiple lines.

[vvanglro]

Here is some discussion about from __future__ import annotations: https://stackoverflow.com/questions/61544854/from-future-import-annotations https://dev.to/tiangolo/the-future-of-fastapi-and-pydantic-is-bright-3pbm

[vvanglro]

There is another motivation because of this. smile Tips for pylance in vscode:

It should just not be split into multiple lines.

Beautiful misunderstanding😂

[frederikaalund]

I think this is a general method of type hinting, whether it is python3.7 or later

I wish that was true! However, the "recommended" approach to type annotations change depending on the target python version. At least in my opinion. Here is an quick primer:

Untyped (python 3.4 and below)

self._pending_subscribes = {}

Typed (python 3.5; when we first got the typing module)

import paho.mqtt.client as mqtt
import asyncio
from typing import Dict, List, Tuple  # These were deprecated in 3.9 in favor of `dict`, `list` and `tuple`.
from typing import Union

self._pending_subscribes: Dict[int, asyncio.Future[Union[Tuple[int, ...], List[mqtt.ReasonCodes]]]] = {}

Quite verbose! 😅

Typed (python 3.9; built-ins now supports subscripting via [])

import paho.mqtt.client as mqtt
import asyncio
from typing import Union

self._pending_subscribes: dict[int, asyncio.Future[Union[tuple[int, ...], list[mqtt.ReasonCodes]]]] = {}

Looks a bit more natural but still quite verbose.

Typed (python 3.10; Unions via |)

import paho.mqtt.client as mqtt
import asyncio
from typing import Union

self._pending_subscribes: dict[int, asyncio.Future[tuple[int, ...] | list[mqtt.ReasonCodes]]] = {}

This is the state of the art. :) At least as far as I know. 😅

Sooo, if we want to do state-of-the-art typing, we need Python 3.10 and above. Dropping support for python 3.7 alone doesn't really give us much in terms of typing.

We can do tricks to get new-style type annotations in older python versions. E.g., to wrap the annotations in strings.

Basically, what I see that this PR accomplishes is to make asyncio-mqtt compatible with python 3.7 via the use of old-style (e.g., pre-3.9) typing. I say 3.7 (and not 3.5) because we use other, non-typing features of 3.7 so that's the minimum version as of now.

Am I missing something or is this the intend of this PR? :) Let me know if I'm wrong!

---

And the delay detection is removed. Personally, I don't like to use delay detection very much.

What do you mean by this mean? :) I can't find the relevant code change (it's all typing related).

[vvanglro]

Yes, I went and read PEP585 and currently the project is using the latest type hinting methods from py39 upwards.

List and Dict in typing will be removed until the end of py39 life. At least this is the plan in the PEP585 proposal.

At present, there are still types such as List Dict Union in py37-11, So I say this is a general type hint.

I call it latency detection:

from __future__ import annotations

[frederikaalund]

I call it latency detection:

from __future__ import annotations

Ah, now it makes sense. I guess that we used it as a workaround. That is, so we can have nice, state-of-the-art typing (ala 3.9 and above) while being backwards-compatible with 3.7. It's not consistent though. I wonder if from __future__ import annotations has the same effect (gives us backwards compatibility but without the strings). That would be a lot cleaner!

[vvanglro]

from __future__ import annotations

Importing it only works in this case:

Import it and include type hints with strings to make some higher version hints available for lower versions.

But this is only for static type checking, Doesn't work at runtime.

It is explained in this article: https://dev.to/tiangolo/the-future-of-fastapi-and-pydantic-is-bright-3pbm

Refer to:

1. https://peps.python.org/pep-0563/
2. https://peps.python.org/pep-0649/

[frederikaalund]

Thanks for linking the tiangolo article. I remember when I first saw that issue on the pydantic bug tracker way back.

But this is only for static type checking, Doesn't work at runtime.

Doesn't work if you do advanced stuff like declaring the class inside a function. For our use case of "runtime types" via dataclass it works just fine. That's because we declare our dataclasses on the top level (globally).

In any case, I vote that we (asyncio-mqtt team):

• Use starte-of-the-art type annotations as per the python 3.10 style (this is the status quo).
• Keep from __future__ import annotations to be backwards compatible with earlier python versions (below 3.10)
• On a case-by-case basis: Wrap our type annotations in strings to make IDEs, type checkers, linters, etc. happy.

@JonathanPlasse and @empicano: What do you think?

On the issue of runtime tying

There is no runtime problem for us (asyncio-mqtt) because we have a simple use case for runtime typing: top-level dataclasses. E.g.:

@dataclass(frozen=True)
class TLSParameters:
    ca_certs: str | None = None  # python 3.10 style type annotations
    ...

I just did a test on python 3.7 to verify that the type hints are indeed backwards compatible:

fpa@sbt-fpa-desktop1:/projects/asyncio-mqtt$ poetry run python -c "import sys; print(sys.version); from asyncio_mqtt.client import TLSParameters; x = TLSParameters(ca_certs='something'); print(x)"
3.7.16 (default, Dec  7 2022, 01:12:19) 
[GCC 9.4.0]
TLSParameters(ca_certs='something', certfile=None, keyfile=None, cert_reqs=None, tls_version=None, ciphers=None, keyfile_password=None)

It works! Python 3.7 magically understands the 3.10-style type hints. Honestly, I don't understand why (I can't find any mention in the python changelog for 3.7x) but I'm glad it works. If I remove the from __future__ import annotations like, I get the expected error:

fpa@sbt-fpa-desktop1:/projects/asyncio-mqtt$ poetry run python -c "import sys; print(sys.version); from asyncio_mqtt.client import TLSParameters; x = TLSParameters(ca_certs='something'); print(x)"
3.7.16 (default, Dec  7 2022, 01:12:19) 
[GCC 9.4.0]
Traceback (most recent call last):
  File "<string>", line 1, in <module>
  File "/projects/asyncio-mqtt/asyncio_mqtt/__init__.py", line 3, in <module>
    from .client import (
  File "/projects/asyncio-mqtt/asyncio_mqtt/client.py", line 58, in <module>
    @dataclass(frozen=True)
  File "/projects/asyncio-mqtt/asyncio_mqtt/client.py", line 60, in TLSParameters
    ca_certs: str | None = None
TypeError: unsupported operand type(s) for |: 'type' and 'NoneType'

[vvanglro]

OK. The type hints of the two writing methods are both acceptable. Can close this PR at any time.

[JonathanPlasse]

The CI runs for all current Python versions. It works on Python 3.7 because the types are never evaluated as types but as strings.

[frederikaalund]

It works on Python 3.7 because the types are never evaluated as types but as strings.

Still doesn't explain why the 3.10 style type hints work for the runtime dataclasses in 3.7. I mean, the dataclass must inspect the type (stringified or not) to deduce how to parse it's arguments. It does work in 3.7 but it's an undocumented feature as far as I can see. :)

[JonathanPlasse]

All annotations with from __future__ import annotations become strings. Dataclasses do not generally inspect types https://docs.python.org/3/library/dataclasses.html#class-variables.

[frederikaalund]

Ah, you're right. 👍 I thought dataclasses where more pydantic.BaseModel-like than they actually are. 😅 Now it all makes sense. :)