Policy for PEP 585 and 604 usage in type annotations

[cdce8p]

Context

We recently updated the mypy version to 0.800, https://github.com/home-assistant/core/pull/45485. With that it would now be possible to use list instead of typing.List -> PEP 585 and int | None instead of typing.Optional[int] -> PEP 604.

There is a caveat though which is why I wanted to discuss it first.

PEP 585 normally requires Python 3.9 and PEP 604 would require 3.10. The use in versions prior to that, the current default is 3.8, is only possible with postponed evaluation of type annotations, PEP 563. This can be enabled starting in 3.7 with from __future__ import annotations. However this workaround also means that any dynamic evaluation will throw an error. Some common examples are the use of typing.get_type_hints or eval.

Proposal

Allow and/or encourage the use of PEP 585 and PEP 604 for type annotations. For an overview on what's possible and what isn't, see the appendix.

Consequences

1. We can't use any functions that use dynamic evaluation in combination with the new syntax until 3.10 is the min required version. This would also apply to all custom_components. Response: At the moment, Home Assistant doesn't use get_type_hints or eval anywhere in the codebase. Furthermore we use type annotations mainly as an additional helper to catch errors early, so I don't expect we'll use any of those anytime soon.

2. from __future__ import annotations needs to be added to every file in which the new syntax should be used. Response: We can easily remove it once 3.10 is the default. In some cases we already use this import to delay the type evaluation.

3. Starting with the new syntax now is future proof. At some point we will need to use list, since typing.List is deprecated as of 3.9.

4. In addition to mypy, I know that Pylance also supports it (for all devs the use VS Code). Other type checker might as well.

5. The standard-library types are arguably more elegant since you don't need an additional import and int | None is IMO easier to read then Optional[int].

Links

https://mypy-lang.blogspot.com/2021/01/mypy-0800-released.html https://mypy.readthedocs.io/en/latest/runtime_troubles.html#future-annotations-import-pep-563 https://www.python.org/dev/peps/pep-0563/ https://www.python.org/dev/peps/pep-0585/ https://www.python.org/dev/peps/pep-0604/

Appendix

The following code will work starting with Python 3.7

from __future__ import annotations

var: int | None  # instead of 'Optional[int]'
var2: int | str  # instead of 'Union[int, str]'
custom_list: list[str]  # instead of 'typing.List[str]'

def func(var: int | list[str]) -> set[int]:
    ...

The following would work in theory with 3.7, however mypy doesn't support it yet

from __future__ import annotations

from typing_extensions import TypeAlias

alias: TypeAlias = "int | None"
var: alias = 42

Lastly, the following will NOT work.

from __future__ import annotations

alias = int | None
# This requires dynamic evaluation, thus '3.9' and '3.10'  respectively.

[frenck]

There is another case where this is useful

from __future__ import annotations

class SomethingSomething:
    def something_else(self) -> SomethingSomething:
        pass

[frenck]

I've been using it myself actually, even in Home Assistant.

It has my vote 👍

[balloob]

I like this a lot.

[frenck]

So did a small change in the linked PR of WLED:

Seems like pylint doesn't like it at this point

[frenck]

Reference issues pylint: https://github.com/PyCQA/pylint/issues/3320

[cdce8p]

Reference issues pylint: PyCQA/pylint#3320

Good catch. Unfortunately we can't use Python 3.9 as pylint environment either. With that Union, Optional and NamedTuple would wrongly be marked as unsubsciptable. The issue is already fixed, but there are still some other ones blocking the next release. So no idea when it will be usable 🤷🏻‍♂️ https://github.com/PyCQA/pylint/issues/3882

We could temporarily disable unsubscriptable-object. However we might miss some other errors. I don't know if it's worth it.

At least the alternative Union Syntax PEP 604 seem to work fine.

[frenck]

Agree, I think we should move forward on PEP 563 & 604 and leave PEP 585 hanging until PyLint releases a fix.

However, running Pylint on Python 3.9 might not be a good idea, as that kinda protects us from getting Python 3.9 features sneaked in (e.g., hostname.removesuffix('.local') 😓 )

[cdce8p]

I've opened a PR for pylint that should fix it. It's already approved, just waiting for an unrelated issue to get fixed before it will be merged. https://github.com/PyCQA/pylint/pull/4059

I expect it will be included in the next release. Just don't know when that will happen 🤷🏻

[cdce8p]

New versions of pylint and astroid were just released that include support for PEP 585 and 604. It should also detect if the new syntax is used incorrectly. https://github.com/home-assistant/core/pull/47205

I think I covered all cases during implementation in pylint, but if you notice any bugs/issues, please tag me. Maybe I can help fix them.

[frenck]

Nice! Happy astroid got fixed 👯

So, this means this path is now a possibility and a go! (I would say).

[cdce8p]

So, this means this path is now a possibility and a go! (I would say).

I would suggest to wait with any larger changes, it would probably make any bugfix releases for 2021.03 a lot more difficult. Additionally, I can investigate if it's possible to automate that process. The path there would probably be:

1. Automatically adding from __future__ import annotations to each module
2. Using pyupgrade to convert the old syntax

I'll report back.

--

One last note: For PEP 604 (alternative Union syntax) I would only suggest enforcing the change from Optional[int] to int | None (etc). The use of a Union is often more deliberate. Although that shouldn't mean that we shouldn't change it at all if it makes sense.

[cdce8p]

I've written a tool to update the typing syntax more or less automatically. https://github.com/cdce8p/python-typing-update/ My plan, unless decided otherwise, would be to split those changes up in 20 - 50 files each, so that the PRs don't get too big. This hopefully also means that I don't have to rebase them that often.

[frenck]

Nice use of tools there @cdce8p 👍

Will try to keep up with you 😉

[cdce8p]

@frenck Just let me know and I'll open some more 😉 Should be somewhere around 15 - 20 PRs.

As for the review. Personally I do trust pylint to detect most (if not all) errors. If it won't, I wouldn't have done a good job implementing them. Before I open a new PR, I only scroll through the changes to see if there is some obvious error. Sometimes my tool has some issues with comments inside the import block, so that they might get removed. Although it detects that and notifies me. These are the manual updates. Besides that there is only formatting which might look strange sometimes (especially for multiline type annotations).

[cdce8p]

All typing update PRs have been merged now, just the last pre-commit updates remaining. Thanks @frenck for taking the time to review them all. Since this is implemented now, shall we close it?

[frenck]

Yep, considering it done!

Thanks for all the work done here! (And not just on the Home Assistant side of things). Well done! 🎖️

Go ahead and close it.

[cdce8p]

Finally done 🚀

[scop]

Yup, great stuff :+1: