O wow. nice feature of isort :) TIL. On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre <pierrejb...@gmail.com> wrote:
> Hello, > > I also like the postponed annotation evaluation. Getting rid of most > forward references is by itself a strong motivation for me, getting > performance improvement on top of that is also great. > > Regarding concerns about PEP 649 superseding PEP 543, I do also agree that > this change would not be this complicated to make. The SC is even > considering rewiring the `from __future__ import annotations` to enable pep > 649 in case it's favored over 543. > > *isort* can enforce automatic imports addition or deletion via the > *add_imports* and *remove_imports* options. (might be good to check if > this can help achieve what we want): > https://pycqa.github.io/isort/docs/configuration/options.html > > Best, > Pierre > > Le lun. 4 juil. 2022 à 20:06, Jarek Potiuk <pot...@apache.org> a écrit : > >> Yeah. I certainly do not want to merge it now in the form it is (also TP >> mentioned that in the comment that in this form it is impossible to >> review). This is more POC but it can be done 'better' - for example the >> empty __init__.py do not have to have the __tuture__ annotations added. I >> think - if the feedback will be general "we like it" then we can also >> introduce it in stages (folder-by-folder) to get it reviewable. >> >> J. >> >> On Mon, Jul 4, 2022 at 5:54 PM Ash Berlin-Taylor <a...@apache.org> wrote: >> >>> I certainly like the future-annotations and have idly thought about how >>> to do this myself. >>> >>> The only downside I can think of is that it might makes cherry picks >>> back to the 2.3 release branch more complicated in some cases (mostly >>> because we generally try to avoid backporting things that aren't bug fixes.) >>> >>> So maybe we merge this in the prep stage for 2.4 instead of right now? >>> >>> -ash >>> >>> On Mon, Jul 4 2022 at 00:12:41 +0200, Jarek Potiuk <ja...@potiuk.com> >>> wrote: >>> >>> Hey everyone, >>> >>> *TL;DR;* I wanted to discuss that maybe (not really sure- but I wanted >>> to check with others) we could migrate the whole codebase of airflow to >>> support PEP 563 - postponed evaluation of annotations. PEP 563 is nicely >>> supported in our case (Python 3.7+) by `from __future__ import annotations` >>> and pyupgrade automated pre-commit does the bulk of the work of converting >>> our code automatically - both current and future. >>> >>> *Is there a POC?* >>> >>> Yeah. I have a PR that implements it. I actually implemented it today - >>> literally in a few hours, in-between of other things. >>> >>> * On one hand it is huge= 3625 files (+16,459 −11,989 lines) >>> * On the other hand it really only **really** changes type annotations >>> (so no real code impact) and those changes are semi-automated. I >>> literally got it "generated" in a couple of hours - and I can easily either >>> rebase or reapply it on top of new changes. >>> >>> There are some spelling mistakes to fix and few tests to fix, but this >>> should be easy. >>> >>> PR here: https://github.com/apache/airflow/pull/24816 >>> >>> *Why ?* >>> >>> Our code gets more and more type-annotated and it's cool. But it does >>> not come for free (for now). The types for functions/classes are evaluated >>> at import time and it takes some computational effort. Also we have a >>> number of cases where we have "string" forward references. This is ugly and >>> does not get as good support for auto-completions and IDEs in general. >>> >>> PEP 563 solves both problems. There is an alternative PEP 649 that could >>> be chosen in the future, but I think we can get some benefits now without >>> waiting for a decision (which even if it comes, will be only implemented in >>> 3.11 or maybe in 3.12 and in the meantime we could get some benefits of PEP >>> 563). >>> >>> 1) Runtime performance is improved in case we do not use type-hints at >>> runtime (we don't) - static type checkers work as usual, but at runtime >>> those annotations are placed in _annotations_ dictionary and evaluated only >>> when runtime type-hint checking is used. >>> >>> 2) String forward references are not needed. >>> No longer `-> 'Type' when you define the type a few lines above, simple >>> `-> Type` will do. >>> >>> Those two on its own would be worth it IMHO, but there is more. >>> >>> 3) the new syntax of types from Python 3.10 is available and it's quite >>> a bit nicer when it comes to Optional usage. Example: >>> >>> CLIENT_AUTH: Optional[Union[Tuple[str, str], Any]] = None >>> >>> turns into: >>> >>> CLIENT_AUTH: tuple[str, str] | Any | None = None >>> >>> 4) Interestingly enough - we do not need to do anything to use the new >>> syntax. The pyupgrade which we have in our pre-commit will automatically >>> convert the "old way" of using Optionals to the new one - as soon as you >>> add `from __future__ import annotations`. This also means that if anyone >>> uses the "old way" in the future, pre-commit will catch it and migrate - >>> this is actually a cool learning experience for all contributors I think as >>> they can learn and get usd to Python 3.10 syntax today. This conversion is >>> done in my PR, you can take a look. >>> >>> 5) It is overwhelmingly mostly backwards-compatible for Python 3.7 >>> except if you used runtime type annotation checking and analysis (which we >>> did not) except inferring multiple outputs in decorators (literally single >>> usage in the entire codebase). And the way we use it seems to be able to >>> continue to work after we add some small fixes - I think just our tests >>> need to be fixed (I think 9 of the failing tests to fix are falling into >>> this camp). >>> >>> *Caveats* >>> >>> Almost none since we are already Python 3.7+. Basically each of our >>> python files must start with: >>> >>> from __future__ import annotations >>> >>> Performance gets improved for imports. The PEP >>> https://peps.python.org/pep-0563/ claims that the overhead for the >>> runtime typing information needed is very small (memory wise) and since we >>> are almost not using runtime type-hint evaluation memory usage will be >>> lower because the type hints will not be evaluated at all during runtime. >>> >>> And similarly, like we do not care about licenses any more (because they >>> are added automatically) by pre-commit, it could be the same with the >>> __future__ import. >>> >>> PEP 649: >>> >>> The caveat is that https://peps.python.org/pep-0649/ - alternative >>> approach to deferring annotation is still being discussed and PEP 563 is >>> not confirmed.. PEP 649 is the reason why PEP 563 wasr removed from Python >>> 3.10 - because there is no decision which one will go forward in the >>> future. Discussion here: >>> https://discuss.python.org/t/type-annotations-pep-649-and-pep-563/11363 >>> >>> However, one could think we are not really affected too much and even if >>> we have to change for 3.11 it will be as simple. We do not use some heavy >>> runtime type-hinting checks, and PEP 0649 `from __future__ import >>> co_annotations' seems to be an easy switch. >>> >>> *How complex is it to migrate* >>> >>> It is simple. We Just merge https://github.com/apache/airflow/pull/24816 >>> and maybe add a pre-commit to force the __future__ import in all our files. >>> That's it. >>> >>> I did it in a couple of hours and I think it is all that we need - the >>> PR I did could likely be merged today and it will be complete. Adding the >>> "__future__" import is very easily automate-able and `pyupgrade` does all >>> the rest. There is one exclusion that we have to make with >>> pyupgrade (gcs.py has "list()" method that interferes with list Type). >>> >>> There were also a few type fixes that had to be done - but nothing major. >>> >>> That's about it. No drama. Some tests/docs need fixing but looking at >>> the changes - very few. We can add pre-commits guarding it. >>> >>> *Options* >>> >>> In the PR I added the 'from __future__ import annotations` to all files >>> (semi-automatically). But it does not have to be like that. One can think >>> that this introduces some "noise". There are plenty of files that do not >>> need it currently. >>> >>> On the other hand there is a nice "property" of having it in all the >>> files: >>> >>> 1) We can even add it in the pre-commit to be injected automatically >>> (should be simple). >>> 2) Pyupgrade will automatically upgrade any future "forward" refs and >>> optionals for us >>> 3) we can keep consistent style of typing everywhere and remove >>> __futures__ when we get to Python 3.10 >>> >>> But we can choose to do half-the way - for example do not add __future__ >>> in empty __init__.py files. Or something similar. >>> >>> I am not 100% convinced myself, but maybe >>> >>> WDYT? Anyone has experience with it? bad/good one? >>> >>> J. >>> >>> >>> >>> >>>
