[DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <ja...@potiuk.com>]

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.

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

Btw. The only reason I even attempted it, was how 'safe' it is. Those are
exclusively changes to typing system - which is only really relevant for
development time rather than runtime. There were literally 2 problematic
changes that I found (and skipped pyupgrade from automatically bumping the
typing)

* Serialization test where we are checking the type of dict at runtime
(this is the only place we do it and only to check for specific Dict type

* GCS hook has 'list' method name that clashes with the list built-in type
and there we have to continue using List (and this one we are not even
going to cherry pick - all the provider's changes (about 70% of cbanges
roughly) we are not even going to attempt to cherry pick at all.

J



niedz., 11 wrz 2022, 14:33 użytkownik Jarek Potiuk <po...@apache.org>
napisał:

> Yeah. It was a bit late. But this was the main reasoning - if we merge
> them quickly AND cherry-pick those to 2.4 branch we have a very good chance
> to avoid any accidental merge of something that was merged since the 2.4
> branch cut-off - there were literally a handful of those PRs merged and it
> will be immediately visible (this is also one reason why I split it into
> many small PRs to detect and fix any kind of problems and remove just
> merged code while cherry-picking).
>
> This was the reasoning behind doing it fast when I recalled that we hold
> off from it in the middle of 2.3 development.
>
> J,.
>
>
> On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> One comment on the timing: but doing it now you make cherry picks for
>> 2.4.x harder, of which were about to do a lot of. The best time to do such
>> a change is just _before_ cutting the next main branch.
>>
>> On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org>
>> wrote:
>>>
>>> Hey Everyone,
>>>
>>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
>>> fully automated PRs I prepared, but a comment from TP made me think that
>>> maybe I went too much ahead and maybe there are other ideas on implementing
>>> the change (or even maybe not implementing it at all).
>>>
>>> More context:
>>>
>>> I have just attempted what I THOUGHT was generally accepted and good
>>> idea, but comment from TP made me think maybe I went ahead of everyone
>>> else, so I am reaching here to see if this was a good idea (And I am ready
>>> to back-out and do it differently - and follow another way if someone can
>>> propose it).
>>>
>>> I just (literally while watching Poland <-> Brazil Volleybal World
>>> Championship semi-finals) prepared 48 separate PRs to migrate to
>>> __future__.annotations which (I thought) was considered a good idea and
>>> something we wanted to defer to after v2-4-test branch cut-off because of
>>> the concerns that it might make cherry-picking more complex. It was 9X%
>>> fully automated conversion - by search/replace and adding `from __future__
>>> import annotations` and letting upgrade do the job (as discussed before).
>>> I also split the one huge PR into 48 (!) much smaller PRs - basically
>>> one per each "airflow" core package, few big providers separately, tests
>>> separately (as they are far less risky than "core code" change).
>>>
>>> We already added __future__ annotation in a number of other places
>>> before - without a "broad" agreement and consensus/voting (different people
>>> did in various places - including myself and others) so I thought this is
>>> not something we need a broader voting/consensus on.
>>>
>>> Again - that was 95% automated change where I contributed mostly the
>>> thinking "how to split '' theI PR and all the rest was literally "git
>>> commit add <package>" (and let pyupgrade and our other pre-commits like
>>> isort and others) do the job.
>>>
>>> It never occured to me that this might be a problem for anyone. My
>>> understanding of the problem was:
>>> * we want to do it
>>> * we were afraid to do it before because it would make cherry-picking to
>>> 2-3 branch more problematic
>>> * we wanted to do it right "when" we cut-off the 2.4 branch (for the
>>> same reason as above - we wanted to avoid cherry-picking problems for 2.4.*
>>> changes)
>>>
>>> So I wanted to make this change as fast as humanly possible - and raise
>>> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
>>> soon as possible to minimise the cherry-picking problem.
>>>
>>> But maybe I was too fast and it's not as straightforward as I thought it
>>> was? (actually it never occured to me that there might be a slightest
>>> problem with it after the earlier discussion). I literally thought about it
>>> as a mechanical change that we want to introduce and that doing that in
>>> small chunks (as discussed in the original PR) was the best approach. The
>>> comment from TP
>>> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849
>>> - made me think that I have misjudged it.
>>>
>>> I wanted to write that email asking all of the committers here to help
>>> to review and merge all those PRs as fast as possible (again - those are
>>> purely mechanical changes, there are literally a few - literally less than
>>> 5) manual changes I had to make to fix some mypy problems.
>>> All the PRs are here:
>>> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>>>
>>> But then I ask for something else then:
>>>
>>> * do you think this is fine we add __future__ annotations in general
>>> * do you think the way I proposed it is somewhat too In-human (i
>>> honestly feel it is an in-human change - because it is fully automated by
>>> our tools :D). But is it wrong approach to have 48 PRs automatically
>>> prepared by our tools to get a "consistent" approach
>>> * did I do it too fast/too early ?
>>> * any ideas on how we can do it differently to avoid the
>>> "cherry-picking" problems (mentioned by others in the original message)?
>>>
>>> I am really concerned to ask if this is something I have done wrongly or
>>> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
>>> approach it differently and ask for consensus on such a flurry of PRS? Or
>>> maybe the timing is wrong.
>>>
>>> I am happy to follow in any way there - I can either keep the PRs and
>>> rebase them until the time is good (it takes about two minutes to rebase
>>> every one of them and apply the automation again)  if the way I proposed it
>>> was because I misunderstood the "sentiment" about the change - or follow
>>> any other way that will help us to implement the change - or even abandon
>>> it completely if we will agree it's not a good idea. I am also happy to
>>> apply any "general comments" there - now that the PR's are there, applying
>>> any automation even on all of them to improve it, is rather simple.
>>>
>>> Looking forward to any comments. I am really curious how others look at
>>> it - regardless of the outcome - always eager to learn new things :)
>>>
>>> J.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
>>> tobiaszkedzierski@gmail.com> wrote:
>>>
>>>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
>>>> Pierre for great isort tip :)
>>>>
>>>> BR
>>>> Tobiasz
>>>>
>>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

All merged (I am just about to cherry-pick it CAREFULLY to v2-4).

BTW. I added one more follow-up PR where we are harnessing the power of
isort to add __future__.annotations in the future automatically (Thanks
Pierre for the tip). While for the original PR I preferred to do it via
search/replace to get better control, this one will make sure that all the
future code added will have __future__.annotations added.

PR here: https://github.com/apache/airflow/pull/26383 (I also moved the
isort configuration to pyproject.toml - this way we have no duplication of
configuration between pre-commit and setup.cfg.

J.


On Mon, Sep 12, 2022 at 1:50 PM Ash Berlin-Taylor <as...@apache.org> wrote:

> Okay, yeah having reviewed the big PR (all providers apart from the big
> three) with over 300 files changes, yeah splitting it out for review was
> the right call!
>
> I have reviewed most of them (and was able to _actually_ review them) and
> am just finishing off the remaining ones now.
>
> -ash
>
> On Sep 12 2022, at 11:05 am, Jarek Potiuk <po...@apache.org> wrote:
>
> Yeah. Combining that in a single commit is easy (I actually had it before
> when I started the discussion https://github.com/apache/airflow/pull/24816
> /.
>
> I only changed it based on TP's comments that it is 'too big to review"
> :).
> But I propose we do it in the way that review is done individually for
> each PR and after that I combine it in a single (or rather two - separate
> one for providers) PR. That would likely be the "best of both worlds".
>
> However, before I proceed with it, I want to do it "properly" and ask
> others. I do not want to make it without the community's consent.
>
> Do you think we should do it? Do you find it as useful as I do ?
>
> One more comment here - the main reason why I wanted to do it is that I
> really do not like is that currently we already have inconsistent approach:
> we currently have 9 packages in "core" airflow that are already using
> __future__.annotations - they've been apparently added without any
> consensus and thinking about consistency: - and I find it terribly annoying
> that sometimes i  have to use (param: list(str) and sometimes params:
> List[str]) - depending on which file I am in.
>
> We currently have __future__.annotaitons added in:
>
> * entry_points.py
> * airflow/__init__.py
> * airflow/models/datasets.py
> * expandinput.py
> * outputs.py
> * templates.py
> * _cron.py
> * timetables/trigger.py
>
> If there will be no strong opposition, I will ask for a lazy consensus and
> I will rebase it and will ask for quick reviews on those 47 PRs and combine
> them into two eventually. I will also cherry-pick them carefully to v2-4
> branch, making sure that I do not drag-along unwanted changes from main.
>
> TP - I hope it all (the discussion and proposal) alleviates your concerns?
>
> J.
>
> On Mon, Sep 12, 2022 at 11:10 AM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> Changes are fine (Other than adding the annotation to empty __init__
> files), but having this as 47 PRs I think is unhelpful when it comes to
> spelunking changes in Git History (i.e. using `git blame`, which I at least
> seem to do once every couple of weeks) so I would prefer it there was 1 for
> core airflow, and one for providers, say.
>
> I don't feel that strongly about this, it's just how I would have done it
> personally -- and having a single rev lets me more easily ignore that rev.
>
> -ash
>
> On Sun, Sep 11 2022 at 14:29:56 +01:00:00, Ash Berlin-Taylor <
> ash@apache.org> wrote:
>
> Right okay, gotcha. I'll take a look tomorrow if no one does before hand.
>
> On 11 September 2022 13:33:27 BST, Jarek Potiuk <po...@apache.org> wrote:
>
> Yeah. It was a bit late. But this was the main reasoning - if we merge
> them quickly AND cherry-pick those to 2.4 branch we have a very good chance
> to avoid any accidental merge of something that was merged since the 2.4
> branch cut-off - there were literally a handful of those PRs merged and it
> will be immediately visible (this is also one reason why I split it into
> many small PRs to detect and fix any kind of problems and remove just
> merged code while cherry-picking).
>
> This was the reasoning behind doing it fast when I recalled that we hold
> off from it in the middle of 2.3 development.
>
> J,.
>
>
> On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
> One comment on the timing: but doing it now you make cherry picks for
> 2.4.x harder, of which were about to do a lot of. The best time to do such
> a change is just _before_ cutting the next main branch.
>
> On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org> wrote:
>
> Hey Everyone,
>
> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
> fully automated PRs I prepared, but a comment from TP made me think that
> maybe I went too much ahead and maybe there are other ideas on implementing
> the change (or even maybe not implementing it at all).
>
> More context:
>
> I have just attempted what I THOUGHT was generally accepted and good idea,
> but comment from TP made me think maybe I went ahead of everyone else, so I
> am reaching here to see if this was a good idea (And I am ready to back-out
> and do it differently - and follow another way if someone can propose it).
>
> I just (literally while watching Poland <-> Brazil Volleybal World
> Championship semi-finals) prepared 48 separate PRs to migrate to
> __future__.annotations which (I thought) was considered a good idea and
> something we wanted to defer to after v2-4-test branch cut-off because of
> the concerns that it might make cherry-picking more complex. It was 9X%
> fully automated conversion - by search/replace and adding `from __future__
> import annotations` and letting upgrade do the job (as discussed before).
> I also split the one huge PR into 48 (!) much smaller PRs - basically one
> per each "airflow" core package, few big providers separately, tests
> separately (as they are far less risky than "core code" change).
>
> We already added __future__ annotation in a number of other places before
> - without a "broad" agreement and consensus/voting (different people did in
> various places - including myself and others) so I thought this is not
> something we need a broader voting/consensus on.
>
> Again - that was 95% automated change where I contributed mostly the
> thinking "how to split '' theI PR and all the rest was literally "git
> commit add <package>" (and let pyupgrade and our other pre-commits like
> isort and others) do the job.
>
> It never occured to me that this might be a problem for anyone. My
> understanding of the problem was:
> * we want to do it
> * we were afraid to do it before because it would make cherry-picking to
> 2-3 branch more problematic
> * we wanted to do it right "when" we cut-off the 2.4 branch (for the same
> reason as above - we wanted to avoid cherry-picking problems for 2.4.*
> changes)
>
> So I wanted to make this change as fast as humanly possible - and raise
> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
> soon as possible to minimise the cherry-picking problem.
>
> But maybe I was too fast and it's not as straightforward as I thought it
> was? (actually it never occured to me that there might be a slightest
> problem with it after the earlier discussion). I literally thought about it
> as a mechanical change that we want to introduce and that doing that in
> small chunks (as discussed in the original PR) was the best approach. The
> comment from TP
> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
> made me think that I have misjudged it.
>
> I wanted to write that email asking all of the committers here to help to
> review and merge all those PRs as fast as possible (again - those are
> purely mechanical changes, there are literally a few - literally less than
> 5) manual changes I had to make to fix some mypy problems.
> All the PRs are here:
> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>
> But then I ask for something else then:
>
> * do you think this is fine we add __future__ annotations in general
> * do you think the way I proposed it is somewhat too In-human (i honestly
> feel it is an in-human change - because it is fully automated by our tools
> :D). But is it wrong approach to have 48 PRs automatically prepared by our
> tools to get a "consistent" approach
> * did I do it too fast/too early ?
> * any ideas on how we can do it differently to avoid the "cherry-picking"
> problems (mentioned by others in the original message)?
>
> I am really concerned to ask if this is something I have done wrongly or
> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
> approach it differently and ask for consensus on such a flurry of PRS? Or
> maybe the timing is wrong.
>
> I am happy to follow in any way there - I can either keep the PRs and
> rebase them until the time is good (it takes about two minutes to rebase
> every one of them and apply the automation again)  if the way I proposed it
> was because I misunderstood the "sentiment" about the change - or follow
> any other way that will help us to implement the change - or even abandon
> it completely if we will agree it's not a good idea. I am also happy to
> apply any "general comments" there - now that the PR's are there, applying
> any automation even on all of them to improve it, is rather simple.
>
> Looking forward to any comments. I am really curious how others look at it
> - regardless of the outcome - always eager to learn new things :)
>
> J.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
> tobiaszkedzierski@gmail.com> wrote:
>
> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
> Pierre for great isort tip :)
>
> BR
> Tobiasz
>
>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

Okay, yeah having reviewed the big PR (all providers apart from the big three) with over 300 files changes, yeah splitting it out for review was the right call!

I have reviewed most of them (and was able to _actually_ review them) and am just finishing off the remaining ones now.
-ash
On Sep 12 2022, at 11:05 am, Jarek Potiuk <po...@apache.org> wrote:
> Yeah. Combining that in a single commit is easy (I actually had it before when I started the discussion https://github.com/apache/airflow/pull/24816/.
>
> I only changed it based on TP's comments that it is 'too big to review" :).
> But I propose we do it in the way that review is done individually for each PR and after that I combine it in a single (or rather two - separate one for providers) PR. That would likely be the "best of both worlds".
>
> However, before I proceed with it, I want to do it "properly" and ask others. I do not want to make it without the community's consent.
>
> Do you think we should do it? Do you find it as useful as I do ?
>
> One more comment here - the main reason why I wanted to do it is that I really do not like is that currently we already have inconsistent approach: we currently have 9 packages in "core" airflow that are already using __future__.annotations - they've been apparently added without any consensus and thinking about consistency: - and I find it terribly annoying that sometimes i have to use (param: list(str) and sometimes params: List[str]) - depending on which file I am in.
>
> We currently have __future__.annotaitons added in:
>
> * entry_points.py
> * airflow/__init__.py
> * airflow/models/datasets.py
> * expandinput.py
> * outputs.py
> * templates.py
> * _cron.py
> * timetables/trigger.py
>
> If there will be no strong opposition, I will ask for a lazy consensus and I will rebase it and will ask for quick reviews on those 47 PRs and combine them into two eventually. I will also cherry-pick them carefully to v2-4 branch, making sure that I do not drag-along unwanted changes from main.
>
> TP - I hope it all (the discussion and proposal) alleviates your concerns?
>
> J.
> On Mon, Sep 12, 2022 at 11:10 AM Ash Berlin-Taylor <ash@apache.org (mailto:ash@apache.org)> wrote:
> > Changes are fine (Other than adding the annotation to empty __init__ files), but having this as 47 PRs I think is unhelpful when it comes to spelunking changes in Git History (i.e. using `git blame`, which I at least seem to do once every couple of weeks) so I would prefer it there was 1 for core airflow, and one for providers, say.
> >
> > I don't feel that strongly about this, it's just how I would have done it personally -- and having a single rev lets me more easily ignore that rev.
> >
> > -ash
> >
> > On Sun, Sep 11 2022 at 14:29:56 +01:00:00, Ash Berlin-Taylor <ash@apache.org (mailto:ash@apache.org)> wrote:
> > > Right okay, gotcha. I'll take a look tomorrow if no one does before hand.
> > >
> > > On 11 September 2022 13:33:27 BST, Jarek Potiuk <potiuk@apache.org (mailto:potiuk@apache.org)> wrote:
> > > > Yeah. It was a bit late. But this was the main reasoning - if we merge them quickly AND cherry-pick those to 2.4 branch we have a very good chance to avoid any accidental merge of something that was merged since the 2.4 branch cut-off - there were literally a handful of those PRs merged and it will be immediately visible (this is also one reason why I split it into many small PRs to detect and fix any kind of problems and remove just merged code while cherry-picking).
> > > >
> > > > This was the reasoning behind doing it fast when I recalled that we hold off from it in the middle of 2.3 development.
> > > >
> > > > J,.
> > > >
> > > >
> > > > On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <ash@apache.org (mailto:ash@apache.org)> wrote:
> > > > > One comment on the timing: but doing it now you make cherry picks for 2.4.x harder, of which were about to do a lot of. The best time to do such a change is just _before_ cutting the next main branch.
> > > > >
> > > > > On 10 September 2022 21:38:12 BST, Jarek Potiuk <potiuk@apache.org (mailto:potiuk@apache.org)> wrote:
> > > > > > Hey Everyone,
> > > > > >
> > > > > > TL;DR; I planned to ask for help in reviewing the 48 (pretty much fully automated PRs I prepared, but a comment from TP made me think that maybe I went too much ahead and maybe there are other ideas on implementing the change (or even maybe not implementing it at all).
> > > > > >
> > > > > > More context:
> > > > > >
> > > > > > I have just attempted what I THOUGHT was generally accepted and good idea, but comment from TP made me think maybe I went ahead of everyone else, so I am reaching here to see if this was a good idea (And I am ready to back-out and do it differently - and follow another way if someone can propose it).
> > > > > >
> > > > > > I just (literally while watching Poland <-> Brazil Volleybal World Championship semi-finals) prepared 48 separate PRs to migrate to __future__.annotations which (I thought) was considered a good idea and something we wanted to defer to after v2-4-test branch cut-off because of the concerns that it might make cherry-picking more complex. It was 9X% fully automated conversion - by search/replace and adding `from __future__ import annotations` and letting upgrade do the job (as discussed before).
> > > > > > I also split the one huge PR into 48 (!) much smaller PRs - basically one per each "airflow" core package, few big providers separately, tests separately (as they are far less risky than "core code" change).
> > > > > >
> > > > > > We already added __future__ annotation in a number of other places before - without a "broad" agreement and consensus/voting (different people did in various places - including myself and others) so I thought this is not something we need a broader voting/consensus on.
> > > > > >
> > > > > > Again - that was 95% automated change where I contributed mostly the thinking "how to split '' theI PR and all the rest was literally "git commit add <package>" (and let pyupgrade and our other pre-commits like isort and others) do the job.
> > > > > >
> > > > > > It never occured to me that this might be a problem for anyone. My understanding of the problem was:
> > > > > > * we want to do it
> > > > > > * we were afraid to do it before because it would make cherry-picking to 2-3 branch more problematic
> > > > > > * we wanted to do it right "when" we cut-off the 2.4 branch (for the same reason as above - we wanted to avoid cherry-picking problems for 2.4.* changes)
> > > > > >
> > > > > > So I wanted to make this change as fast as humanly possible - and raise this multiple PRs so we can review/approve (and I'd cherry-pick them) as soon as possible to minimise the cherry-picking problem.
> > > > > >
> > > > > > But maybe I was too fast and it's not as straightforward as I thought it was? (actually it never occured to me that there might be a slightest problem with it after the earlier discussion). I literally thought about it as a mechanical change that we want to introduce and that doing that in small chunks (as discussed in the original PR) was the best approach. The comment from TP https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849 - made me think that I have misjudged it.
> > > > > >
> > > > > > I wanted to write that email asking all of the committers here to help to review and merge all those PRs as fast as possible (again - those are purely mechanical changes, there are literally a few - literally less than 5) manual changes I had to make to fix some mypy problems.
> > > > > > All the PRs are here: https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
> > > > > >
> > > > > >
> > > > > > But then I ask for something else then:
> > > > > >
> > > > > > * do you think this is fine we add __future__ annotations in general
> > > > > > * do you think the way I proposed it is somewhat too In-human (i honestly feel it is an in-human change - because it is fully automated by our tools :D). But is it wrong approach to have 48 PRs automatically prepared by our tools to get a "consistent" approach
> > > > > > * did I do it too fast/too early ?
> > > > > > * any ideas on how we can do it differently to avoid the "cherry-picking" problems (mentioned by others in the original message)?
> > > > > >
> > > > > > I am really concerned to ask if this is something I have done wrongly or misunderstood the consensus, and maybe chose the wrong way ? Maybe I should approach it differently and ask for consensus on such a flurry of PRS? Or maybe the timing is wrong.
> > > > > >
> > > > > > I am happy to follow in any way there - I can either keep the PRs and rebase them until the time is good (it takes about two minutes to rebase every one of them and apply the automation again) if the way I proposed it was because I misunderstood the "sentiment" about the change - or follow any other way that will help us to implement the change - or even abandon it completely if we will agree it's not a good idea. I am also happy to apply any "general comments" there - now that the PR's are there, applying any automation even on all of them to improve it, is rather simple.
> > > > > >
> > > > > > Looking forward to any comments. I am really curious how others look at it - regardless of the outcome - always eager to learn new things :)
> > > > > >
> > > > > > J.
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > >
> > > > > > On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <tobiaszkedzierski@gmail.com (mailto:tobiaszkedzierski@gmail.com)> wrote:
> > > > > > > Great idea. +1 from me. Thanks for raising this up Jarek and thanks to Pierre for great isort tip :)
> > > > > > >
> > > > > > > BR
> > > > > > > Tobiasz
> > > > > > >
> > > > > > >
> > > > > >
> > > > > >
> > > > >
> > > > >
> > > > >
> > > >
> > > >
> > >
> > >
> >
> >
>
>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

Yeah. Combining that in a single commit is easy (I actually had it before
when I started the discussion https://github.com/apache/airflow/pull/24816
/.

I only changed it based on TP's comments that it is 'too big to review" :).
But I propose we do it in the way that review is done individually for each
PR and after that I combine it in a single (or rather two - separate one
for providers) PR. That would likely be the "best of both worlds".

However, before I proceed with it, I want to do it "properly" and ask
others. I do not want to make it without the community's consent.

Do you think we should do it? Do you find it as useful as I do ?

One more comment here - the main reason why I wanted to do it is that I
really do not like is that currently we already have inconsistent approach:
we currently have 9 packages in "core" airflow that are already using
__future__.annotations - they've been apparently added without any
consensus and thinking about consistency: - and I find it terribly annoying
that sometimes i  have to use (param: list(str) and sometimes params:
List[str]) - depending on which file I am in.

We currently have __future__.annotaitons added in:

* entry_points.py
* airflow/__init__.py
* airflow/models/datasets.py
* expandinput.py
* outputs.py
* templates.py
* _cron.py
* timetables/trigger.py

If there will be no strong opposition, I will ask for a lazy consensus and
I will rebase it and will ask for quick reviews on those 47 PRs and combine
them into two eventually. I will also cherry-pick them carefully to v2-4
branch, making sure that I do not drag-along unwanted changes from main.

TP - I hope it all (the discussion and proposal) alleviates your concerns?

J.

On Mon, Sep 12, 2022 at 11:10 AM Ash Berlin-Taylor <as...@apache.org> wrote:

> Changes are fine (Other than adding the annotation to empty __init__
> files), but having this as 47 PRs I think is unhelpful when it comes to
> spelunking changes in Git History (i.e. using `git blame`, which I at least
> seem to do once every couple of weeks) so I would prefer it there was 1 for
> core airflow, and one for providers, say.
>
> I don't feel that strongly about this, it's just how I would have done it
> personally -- and having a single rev lets me more easily ignore that rev.
>
> -ash
>
> On Sun, Sep 11 2022 at 14:29:56 +01:00:00, Ash Berlin-Taylor <
> ash@apache.org> wrote:
>
> Right okay, gotcha. I'll take a look tomorrow if no one does before hand.
>
> On 11 September 2022 13:33:27 BST, Jarek Potiuk <po...@apache.org> wrote:
>>
>> Yeah. It was a bit late. But this was the main reasoning - if we merge
>> them quickly AND cherry-pick those to 2.4 branch we have a very good chance
>> to avoid any accidental merge of something that was merged since the 2.4
>> branch cut-off - there were literally a handful of those PRs merged and it
>> will be immediately visible (this is also one reason why I split it into
>> many small PRs to detect and fix any kind of problems and remove just
>> merged code while cherry-picking).
>>
>> This was the reasoning behind doing it fast when I recalled that we hold
>> off from it in the middle of 2.3 development.
>>
>> J,.
>>
>>
>> On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>>
>>> One comment on the timing: but doing it now you make cherry picks for
>>> 2.4.x harder, of which were about to do a lot of. The best time to do such
>>> a change is just _before_ cutting the next main branch.
>>>
>>> On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org>
>>> wrote:
>>>>
>>>> Hey Everyone,
>>>>
>>>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
>>>> fully automated PRs I prepared, but a comment from TP made me think that
>>>> maybe I went too much ahead and maybe there are other ideas on implementing
>>>> the change (or even maybe not implementing it at all).
>>>>
>>>> More context:
>>>>
>>>> I have just attempted what I THOUGHT was generally accepted and good
>>>> idea, but comment from TP made me think maybe I went ahead of everyone
>>>> else, so I am reaching here to see if this was a good idea (And I am ready
>>>> to back-out and do it differently - and follow another way if someone can
>>>> propose it).
>>>>
>>>> I just (literally while watching Poland <-> Brazil Volleybal World
>>>> Championship semi-finals) prepared 48 separate PRs to migrate to
>>>> __future__.annotations which (I thought) was considered a good idea and
>>>> something we wanted to defer to after v2-4-test branch cut-off because of
>>>> the concerns that it might make cherry-picking more complex. It was 9X%
>>>> fully automated conversion - by search/replace and adding `from __future__
>>>> import annotations` and letting upgrade do the job (as discussed before).
>>>> I also split the one huge PR into 48 (!) much smaller PRs - basically
>>>> one per each "airflow" core package, few big providers separately, tests
>>>> separately (as they are far less risky than "core code" change).
>>>>
>>>> We already added __future__ annotation in a number of other places
>>>> before - without a "broad" agreement and consensus/voting (different people
>>>> did in various places - including myself and others) so I thought this is
>>>> not something we need a broader voting/consensus on.
>>>>
>>>> Again - that was 95% automated change where I contributed mostly the
>>>> thinking "how to split '' theI PR and all the rest was literally "git
>>>> commit add <package>" (and let pyupgrade and our other pre-commits like
>>>> isort and others) do the job.
>>>>
>>>> It never occured to me that this might be a problem for anyone. My
>>>> understanding of the problem was:
>>>> * we want to do it
>>>> * we were afraid to do it before because it would make cherry-picking
>>>> to 2-3 branch more problematic
>>>> * we wanted to do it right "when" we cut-off the 2.4 branch (for the
>>>> same reason as above - we wanted to avoid cherry-picking problems for 2.4.*
>>>> changes)
>>>>
>>>> So I wanted to make this change as fast as humanly possible - and raise
>>>> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
>>>> soon as possible to minimise the cherry-picking problem.
>>>>
>>>> But maybe I was too fast and it's not as straightforward as I thought
>>>> it was? (actually it never occured to me that there might be a slightest
>>>> problem with it after the earlier discussion). I literally thought about it
>>>> as a mechanical change that we want to introduce and that doing that in
>>>> small chunks (as discussed in the original PR) was the best approach. The
>>>> comment from TP
>>>> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849
>>>> - made me think that I have misjudged it.
>>>>
>>>> I wanted to write that email asking all of the committers here to help
>>>> to review and merge all those PRs as fast as possible (again - those are
>>>> purely mechanical changes, there are literally a few - literally less than
>>>> 5) manual changes I had to make to fix some mypy problems.
>>>> All the PRs are here:
>>>> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>>>>
>>>> But then I ask for something else then:
>>>>
>>>> * do you think this is fine we add __future__ annotations in general
>>>> * do you think the way I proposed it is somewhat too In-human (i
>>>> honestly feel it is an in-human change - because it is fully automated by
>>>> our tools :D). But is it wrong approach to have 48 PRs automatically
>>>> prepared by our tools to get a "consistent" approach
>>>> * did I do it too fast/too early ?
>>>> * any ideas on how we can do it differently to avoid the
>>>> "cherry-picking" problems (mentioned by others in the original message)?
>>>>
>>>> I am really concerned to ask if this is something I have done wrongly
>>>> or misunderstood the consensus, and maybe chose the wrong way ? Maybe I
>>>> should approach it differently and ask for consensus on such a flurry of
>>>> PRS? Or maybe the timing is wrong.
>>>>
>>>> I am happy to follow in any way there - I can either keep the PRs and
>>>> rebase them until the time is good (it takes about two minutes to rebase
>>>> every one of them and apply the automation again)  if the way I proposed it
>>>> was because I misunderstood the "sentiment" about the change - or follow
>>>> any other way that will help us to implement the change - or even abandon
>>>> it completely if we will agree it's not a good idea. I am also happy to
>>>> apply any "general comments" there - now that the PR's are there, applying
>>>> any automation even on all of them to improve it, is rather simple.
>>>>
>>>> Looking forward to any comments. I am really curious how others look at
>>>> it - regardless of the outcome - always eager to learn new things :)
>>>>
>>>> J.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
>>>> tobiaszkedzierski@gmail.com> wrote:
>>>>
>>>>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks
>>>>> to Pierre for great isort tip :)
>>>>>
>>>>> BR
>>>>> Tobiasz
>>>>>
>>>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

Changes are fine (Other than adding the annotation to empty __init__ 
files), but having this as 47 PRs I think is unhelpful when it comes to 
spelunking changes in Git History (i.e. using `git blame`, which I at 
least seem to do once every couple of weeks) so I would prefer it there 
was 1 for core airflow, and one for providers, say.

I don't feel that strongly about this, it's just how I would have done 
it personally -- and having a single rev lets me more easily ignore 
that rev.

-ash

On Sun, Sep 11 2022 at 14:29:56 +01:00:00, Ash Berlin-Taylor 
<as...@apache.org> wrote:
> Right okay, gotcha. I'll take a look tomorrow if no one does before 
> hand.
> 
> On 11 September 2022 13:33:27 BST, Jarek Potiuk <po...@apache.org> 
> wrote:
>> Yeah. It was a bit late. But this was the main reasoning - if we 
>> merge them quickly AND cherry-pick those to 2.4 branch we have a 
>> very good chance to avoid any accidental merge of something that was 
>> merged since the 2.4 branch cut-off - there were literally a handful 
>> of those PRs merged and it will be immediately visible (this is also 
>> one reason why I split it into many small PRs to detect and fix any 
>> kind of problems and remove just merged code while cherry-picking).
>> 
>> This was the reasoning behind doing it fast when I recalled that we 
>> hold off from it in the middle of 2.3 development.
>> 
>> J,.
>> 
>> 
>> On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <ash@apache.org 
>> <ma...@apache.org>> wrote:
>>> One comment on the timing: but doing it now you make cherry picks 
>>> for 2.4.x harder, of which were about to do a lot of. The best time 
>>> to do such a change is just _before_ cutting the next main branch.
>>> 
>>> On 10 September 2022 21:38:12 BST, Jarek Potiuk <potiuk@apache.org 
>>> <ma...@apache.org>> wrote:
>>>> Hey Everyone,
>>>> 
>>>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much 
>>>> fully automated PRs I prepared, but a comment from TP made me 
>>>> think that maybe I went too much ahead and maybe there are other 
>>>> ideas on implementing the change (or even maybe not implementing 
>>>> it at all).
>>>> 
>>>> More context:
>>>> 
>>>> I have just attempted what I THOUGHT was generally accepted and 
>>>> good idea, but comment from TP made me think maybe I went ahead of 
>>>> everyone else, so I am reaching here to see if this was a good 
>>>> idea (And I am ready to back-out and do it differently - and 
>>>> follow another way if someone can propose it).
>>>> 
>>>> I just (literally while watching Poland <-> Brazil Volleybal World 
>>>> Championship semi-finals) prepared 48 separate PRs to migrate to 
>>>> __future__.annotations which (I thought) was considered a good 
>>>> idea and something we wanted to defer to after v2-4-test branch 
>>>> cut-off because of the concerns that it might make cherry-picking 
>>>> more complex. It was 9X% fully automated conversion - by 
>>>> search/replace and adding `from __future__ import annotations` and 
>>>> letting upgrade do the job (as discussed before).
>>>> I also split the one huge PR into 48 (!) much smaller PRs - 
>>>> basically one per each "airflow" core package, few big providers 
>>>> separately, tests separately (as they are far less risky than 
>>>> "core code" change).
>>>> 
>>>> We already added __future__ annotation in a number of other places 
>>>> before - without a "broad" agreement and consensus/voting 
>>>> (different people did in various places - including myself and 
>>>> others) so I thought this is not something we need a broader 
>>>> voting/consensus on.
>>>> 
>>>> Again - that was 95% automated change where I contributed mostly 
>>>> the thinking "how to split '' theI PR and all the rest was 
>>>> literally "git commit add <package>" (and let pyupgrade and our 
>>>> other pre-commits like isort and others) do the job.
>>>> 
>>>> It never occured to me that this might be a problem for anyone. My 
>>>> understanding of the problem was:
>>>> * we want to do it
>>>> * we were afraid to do it before because it would make 
>>>> cherry-picking to 2-3 branch more problematic
>>>> * we wanted to do it right "when" we cut-off the 2.4 branch (for 
>>>> the same reason as above - we wanted to avoid cherry-picking 
>>>> problems for 2.4.* changes)
>>>> 
>>>> So I wanted to make this change as fast as humanly possible - and 
>>>> raise this multiple PRs so we can review/approve (and I'd 
>>>> cherry-pick them) as soon as possible to minimise the 
>>>> cherry-picking problem.
>>>> 
>>>> But maybe I was too fast and it's not as straightforward as I 
>>>> thought it was? (actually it never occured to me that there might 
>>>> be a slightest problem with it after the earlier discussion). I 
>>>> literally thought about it as a mechanical change that we want to 
>>>> introduce and that doing that in small chunks (as discussed in the 
>>>> original PR) was the best approach. The comment from TP 
>>>> <https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849> 
>>>>  - made me think that I have misjudged it.
>>>> 
>>>> I wanted to write that email asking all of the committers here to 
>>>> help to review and merge all those PRs as fast as possible (again 
>>>> - those are purely mechanical changes, there are literally a few - 
>>>> literally less than 5) manual changes I had to make to fix some 
>>>> mypy problems.
>>>> All the PRs are here: 
>>>> <https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations>
>>>> 
>>>> But then I ask for something else then:
>>>> 
>>>> * do you think this is fine we add __future__ annotations in 
>>>> general
>>>> * do you think the way I proposed it is somewhat too In-human (i 
>>>> honestly feel it is an in-human change - because it is fully 
>>>> automated by our tools :D). But is it wrong approach to have 48 
>>>> PRs automatically prepared by our tools to get a "consistent" 
>>>> approach
>>>> * did I do it too fast/too early ?
>>>> * any ideas on how we can do it differently to avoid the 
>>>> "cherry-picking" problems (mentioned by others in the original 
>>>> message)?
>>>> 
>>>> I am really concerned to ask if this is something I have done 
>>>> wrongly or misunderstood the consensus, and maybe chose the wrong 
>>>> way ? Maybe I should approach it differently and ask for consensus 
>>>> on such a flurry of PRS? Or maybe the timing is wrong.
>>>> 
>>>> I am happy to follow in any way there - I can either keep the PRs 
>>>> and rebase them until the time is good (it takes about two minutes 
>>>> to rebase every one of them and apply the automation again)  if 
>>>> the way I proposed it was because I misunderstood the "sentiment" 
>>>> about the change - or follow any other way that will help us to 
>>>> implement the change - or even abandon it completely if we will 
>>>> agree it's not a good idea. I am also happy to apply any "general 
>>>> comments" there - now that the PR's are there, applying any 
>>>> automation even on all of them to improve it, is rather simple.
>>>> 
>>>> Looking forward to any comments. I am really curious how others 
>>>> look at it - regardless of the outcome - always eager to learn new 
>>>> things :)
>>>> 
>>>> J.
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> 
>>>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski 
>>>> <tobiaszkedzierski@gmail.com <ma...@gmail.com>> 
>>>> wrote:
>>>>> Great idea.  +1 from me. Thanks for raising this up Jarek and 
>>>>> thanks to Pierre for great isort tip :)
>>>>> 
>>>>> BR
>>>>> Tobiasz

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

Right okay, gotcha. I'll take a look tomorrow if no one does before hand.

On 11 September 2022 13:33:27 BST, Jarek Potiuk <po...@apache.org> wrote:
>Yeah. It was a bit late. But this was the main reasoning - if we merge them
>quickly AND cherry-pick those to 2.4 branch we have a very good chance to
>avoid any accidental merge of something that was merged since the 2.4
>branch cut-off - there were literally a handful of those PRs merged and it
>will be immediately visible (this is also one reason why I split it into
>many small PRs to detect and fix any kind of problems and remove just
>merged code while cherry-picking).
>
>This was the reasoning behind doing it fast when I recalled that we hold
>off from it in the middle of 2.3 development.
>
>J,.
>
>
>On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <as...@apache.org> wrote:
>
>> One comment on the timing: but doing it now you make cherry picks for
>> 2.4.x harder, of which were about to do a lot of. The best time to do such
>> a change is just _before_ cutting the next main branch.
>>
>> On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org> wrote:
>>>
>>> Hey Everyone,
>>>
>>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
>>> fully automated PRs I prepared, but a comment from TP made me think that
>>> maybe I went too much ahead and maybe there are other ideas on implementing
>>> the change (or even maybe not implementing it at all).
>>>
>>> More context:
>>>
>>> I have just attempted what I THOUGHT was generally accepted and good
>>> idea, but comment from TP made me think maybe I went ahead of everyone
>>> else, so I am reaching here to see if this was a good idea (And I am ready
>>> to back-out and do it differently - and follow another way if someone can
>>> propose it).
>>>
>>> I just (literally while watching Poland <-> Brazil Volleybal World
>>> Championship semi-finals) prepared 48 separate PRs to migrate to
>>> __future__.annotations which (I thought) was considered a good idea and
>>> something we wanted to defer to after v2-4-test branch cut-off because of
>>> the concerns that it might make cherry-picking more complex. It was 9X%
>>> fully automated conversion - by search/replace and adding `from __future__
>>> import annotations` and letting upgrade do the job (as discussed before).
>>> I also split the one huge PR into 48 (!) much smaller PRs - basically one
>>> per each "airflow" core package, few big providers separately, tests
>>> separately (as they are far less risky than "core code" change).
>>>
>>> We already added __future__ annotation in a number of other places before
>>> - without a "broad" agreement and consensus/voting (different people did in
>>> various places - including myself and others) so I thought this is not
>>> something we need a broader voting/consensus on.
>>>
>>> Again - that was 95% automated change where I contributed mostly the
>>> thinking "how to split '' theI PR and all the rest was literally "git
>>> commit add <package>" (and let pyupgrade and our other pre-commits like
>>> isort and others) do the job.
>>>
>>> It never occured to me that this might be a problem for anyone. My
>>> understanding of the problem was:
>>> * we want to do it
>>> * we were afraid to do it before because it would make cherry-picking to
>>> 2-3 branch more problematic
>>> * we wanted to do it right "when" we cut-off the 2.4 branch (for the same
>>> reason as above - we wanted to avoid cherry-picking problems for 2.4.*
>>> changes)
>>>
>>> So I wanted to make this change as fast as humanly possible - and raise
>>> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
>>> soon as possible to minimise the cherry-picking problem.
>>>
>>> But maybe I was too fast and it's not as straightforward as I thought it
>>> was? (actually it never occured to me that there might be a slightest
>>> problem with it after the earlier discussion). I literally thought about it
>>> as a mechanical change that we want to introduce and that doing that in
>>> small chunks (as discussed in the original PR) was the best approach. The
>>> comment from TP
>>> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
>>> made me think that I have misjudged it.
>>>
>>> I wanted to write that email asking all of the committers here to help to
>>> review and merge all those PRs as fast as possible (again - those are
>>> purely mechanical changes, there are literally a few - literally less than
>>> 5) manual changes I had to make to fix some mypy problems.
>>> All the PRs are here:
>>> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>>>
>>> But then I ask for something else then:
>>>
>>> * do you think this is fine we add __future__ annotations in general
>>> * do you think the way I proposed it is somewhat too In-human (i honestly
>>> feel it is an in-human change - because it is fully automated by our tools
>>> :D). But is it wrong approach to have 48 PRs automatically prepared by our
>>> tools to get a "consistent" approach
>>> * did I do it too fast/too early ?
>>> * any ideas on how we can do it differently to avoid the "cherry-picking"
>>> problems (mentioned by others in the original message)?
>>>
>>> I am really concerned to ask if this is something I have done wrongly or
>>> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
>>> approach it differently and ask for consensus on such a flurry of PRS? Or
>>> maybe the timing is wrong.
>>>
>>> I am happy to follow in any way there - I can either keep the PRs and
>>> rebase them until the time is good (it takes about two minutes to rebase
>>> every one of them and apply the automation again)  if the way I proposed it
>>> was because I misunderstood the "sentiment" about the change - or follow
>>> any other way that will help us to implement the change - or even abandon
>>> it completely if we will agree it's not a good idea. I am also happy to
>>> apply any "general comments" there - now that the PR's are there, applying
>>> any automation even on all of them to improve it, is rather simple.
>>>
>>> Looking forward to any comments. I am really curious how others look at
>>> it - regardless of the outcome - always eager to learn new things :)
>>>
>>> J.
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>>
>>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
>>> tobiaszkedzierski@gmail.com> wrote:
>>>
>>>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
>>>> Pierre for great isort tip :)
>>>>
>>>> BR
>>>> Tobiasz
>>>>
>>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

Yeah. It was a bit late. But this was the main reasoning - if we merge them
quickly AND cherry-pick those to 2.4 branch we have a very good chance to
avoid any accidental merge of something that was merged since the 2.4
branch cut-off - there were literally a handful of those PRs merged and it
will be immediately visible (this is also one reason why I split it into
many small PRs to detect and fix any kind of problems and remove just
merged code while cherry-picking).

This was the reasoning behind doing it fast when I recalled that we hold
off from it in the middle of 2.3 development.

J,.


On Sun, Sep 11, 2022 at 1:19 PM Ash Berlin-Taylor <as...@apache.org> wrote:

> One comment on the timing: but doing it now you make cherry picks for
> 2.4.x harder, of which were about to do a lot of. The best time to do such
> a change is just _before_ cutting the next main branch.
>
> On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org> wrote:
>>
>> Hey Everyone,
>>
>> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
>> fully automated PRs I prepared, but a comment from TP made me think that
>> maybe I went too much ahead and maybe there are other ideas on implementing
>> the change (or even maybe not implementing it at all).
>>
>> More context:
>>
>> I have just attempted what I THOUGHT was generally accepted and good
>> idea, but comment from TP made me think maybe I went ahead of everyone
>> else, so I am reaching here to see if this was a good idea (And I am ready
>> to back-out and do it differently - and follow another way if someone can
>> propose it).
>>
>> I just (literally while watching Poland <-> Brazil Volleybal World
>> Championship semi-finals) prepared 48 separate PRs to migrate to
>> __future__.annotations which (I thought) was considered a good idea and
>> something we wanted to defer to after v2-4-test branch cut-off because of
>> the concerns that it might make cherry-picking more complex. It was 9X%
>> fully automated conversion - by search/replace and adding `from __future__
>> import annotations` and letting upgrade do the job (as discussed before).
>> I also split the one huge PR into 48 (!) much smaller PRs - basically one
>> per each "airflow" core package, few big providers separately, tests
>> separately (as they are far less risky than "core code" change).
>>
>> We already added __future__ annotation in a number of other places before
>> - without a "broad" agreement and consensus/voting (different people did in
>> various places - including myself and others) so I thought this is not
>> something we need a broader voting/consensus on.
>>
>> Again - that was 95% automated change where I contributed mostly the
>> thinking "how to split '' theI PR and all the rest was literally "git
>> commit add <package>" (and let pyupgrade and our other pre-commits like
>> isort and others) do the job.
>>
>> It never occured to me that this might be a problem for anyone. My
>> understanding of the problem was:
>> * we want to do it
>> * we were afraid to do it before because it would make cherry-picking to
>> 2-3 branch more problematic
>> * we wanted to do it right "when" we cut-off the 2.4 branch (for the same
>> reason as above - we wanted to avoid cherry-picking problems for 2.4.*
>> changes)
>>
>> So I wanted to make this change as fast as humanly possible - and raise
>> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
>> soon as possible to minimise the cherry-picking problem.
>>
>> But maybe I was too fast and it's not as straightforward as I thought it
>> was? (actually it never occured to me that there might be a slightest
>> problem with it after the earlier discussion). I literally thought about it
>> as a mechanical change that we want to introduce and that doing that in
>> small chunks (as discussed in the original PR) was the best approach. The
>> comment from TP
>> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
>> made me think that I have misjudged it.
>>
>> I wanted to write that email asking all of the committers here to help to
>> review and merge all those PRs as fast as possible (again - those are
>> purely mechanical changes, there are literally a few - literally less than
>> 5) manual changes I had to make to fix some mypy problems.
>> All the PRs are here:
>> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>>
>> But then I ask for something else then:
>>
>> * do you think this is fine we add __future__ annotations in general
>> * do you think the way I proposed it is somewhat too In-human (i honestly
>> feel it is an in-human change - because it is fully automated by our tools
>> :D). But is it wrong approach to have 48 PRs automatically prepared by our
>> tools to get a "consistent" approach
>> * did I do it too fast/too early ?
>> * any ideas on how we can do it differently to avoid the "cherry-picking"
>> problems (mentioned by others in the original message)?
>>
>> I am really concerned to ask if this is something I have done wrongly or
>> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
>> approach it differently and ask for consensus on such a flurry of PRS? Or
>> maybe the timing is wrong.
>>
>> I am happy to follow in any way there - I can either keep the PRs and
>> rebase them until the time is good (it takes about two minutes to rebase
>> every one of them and apply the automation again)  if the way I proposed it
>> was because I misunderstood the "sentiment" about the change - or follow
>> any other way that will help us to implement the change - or even abandon
>> it completely if we will agree it's not a good idea. I am also happy to
>> apply any "general comments" there - now that the PR's are there, applying
>> any automation even on all of them to improve it, is rather simple.
>>
>> Looking forward to any comments. I am really curious how others look at
>> it - regardless of the outcome - always eager to learn new things :)
>>
>> J.
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
>> tobiaszkedzierski@gmail.com> wrote:
>>
>>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
>>> Pierre for great isort tip :)
>>>
>>> BR
>>> Tobiasz
>>>
>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

One comment on the timing: but doing it now you make cherry picks for 2.4.x harder, of which were about to do a lot of. The best time to do such a change is just _before_ cutting the next main branch.

On 10 September 2022 21:38:12 BST, Jarek Potiuk <po...@apache.org> wrote:
>Hey Everyone,
>
>TL;DR; I planned to ask for help in reviewing the 48 (pretty much
>fully automated PRs I prepared, but a comment from TP made me think that
>maybe I went too much ahead and maybe there are other ideas on implementing
>the change (or even maybe not implementing it at all).
>
>More context:
>
>I have just attempted what I THOUGHT was generally accepted and good idea,
>but comment from TP made me think maybe I went ahead of everyone else, so I
>am reaching here to see if this was a good idea (And I am ready to back-out
>and do it differently - and follow another way if someone can propose it).
>
>I just (literally while watching Poland <-> Brazil Volleybal World
>Championship semi-finals) prepared 48 separate PRs to migrate to
>__future__.annotations which (I thought) was considered a good idea and
>something we wanted to defer to after v2-4-test branch cut-off because of
>the concerns that it might make cherry-picking more complex. It was 9X%
>fully automated conversion - by search/replace and adding `from __future__
>import annotations` and letting upgrade do the job (as discussed before).
>I also split the one huge PR into 48 (!) much smaller PRs - basically one
>per each "airflow" core package, few big providers separately, tests
>separately (as they are far less risky than "core code" change).
>
>We already added __future__ annotation in a number of other places before -
>without a "broad" agreement and consensus/voting (different people did in
>various places - including myself and others) so I thought this is not
>something we need a broader voting/consensus on.
>
>Again - that was 95% automated change where I contributed mostly the
>thinking "how to split '' theI PR and all the rest was literally "git
>commit add <package>" (and let pyupgrade and our other pre-commits like
>isort and others) do the job.
>
>It never occured to me that this might be a problem for anyone. My
>understanding of the problem was:
>* we want to do it
>* we were afraid to do it before because it would make cherry-picking to
>2-3 branch more problematic
>* we wanted to do it right "when" we cut-off the 2.4 branch (for the same
>reason as above - we wanted to avoid cherry-picking problems for 2.4.*
>changes)
>
>So I wanted to make this change as fast as humanly possible - and raise
>this multiple PRs so we can review/approve (and I'd cherry-pick them) as
>soon as possible to minimise the cherry-picking problem.
>
>But maybe I was too fast and it's not as straightforward as I thought it
>was? (actually it never occured to me that there might be a slightest
>problem with it after the earlier discussion). I literally thought about it
>as a mechanical change that we want to introduce and that doing that in
>small chunks (as discussed in the original PR) was the best approach. The
>comment from TP
>https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
>made me think that I have misjudged it.
>
>I wanted to write that email asking all of the committers here to help to
>review and merge all those PRs as fast as possible (again - those are
>purely mechanical changes, there are literally a few - literally less than
>5) manual changes I had to make to fix some mypy problems.
>All the PRs are here:
>https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>
>But then I ask for something else then:
>
>* do you think this is fine we add __future__ annotations in general
>* do you think the way I proposed it is somewhat too In-human (i honestly
>feel it is an in-human change - because it is fully automated by our tools
>:D). But is it wrong approach to have 48 PRs automatically prepared by our
>tools to get a "consistent" approach
>* did I do it too fast/too early ?
>* any ideas on how we can do it differently to avoid the "cherry-picking"
>problems (mentioned by others in the original message)?
>
>I am really concerned to ask if this is something I have done wrongly or
>misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
>approach it differently and ask for consensus on such a flurry of PRS? Or
>maybe the timing is wrong.
>
>I am happy to follow in any way there - I can either keep the PRs and
>rebase them until the time is good (it takes about two minutes to rebase
>every one of them and apply the automation again)  if the way I proposed it
>was because I misunderstood the "sentiment" about the change - or follow
>any other way that will help us to implement the change - or even abandon
>it completely if we will agree it's not a good idea. I am also happy to
>apply any "general comments" there - now that the PR's are there, applying
>any automation even on all of them to improve it, is rather simple.
>
>Looking forward to any comments. I am really curious how others look at it
>- regardless of the outcome - always eager to learn new things :)
>
>J.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
>On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
>tobiaszkedzierski@gmail.com> wrote:
>
>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
>> Pierre for great isort tip :)
>>
>> BR
>> Tobiasz
>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

BTW. All the 47 PRs are "green" now
https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
and
if we want we could just merge them and get __future__. annotations applied
automatically.

But I am happy for any direction to either close it or wait with them, or
do it completely differently, or even not do it at all. If need be, I've
learned everything to re-do all the PRs in an even more automated way any
time we decide to do it, so this is not a problem to drop it if needed.

Would love to hear what others think. And apologies if that caused any
distress (to TP but also anyone who could see it as a problem, I was not
aware this might be a problem :).

J.


On Sat, Sep 10, 2022 at 10:38 PM Jarek Potiuk <po...@apache.org> wrote:

> Hey Everyone,
>
> TL;DR; I planned to ask for help in reviewing the 48 (pretty much
> fully automated PRs I prepared, but a comment from TP made me think that
> maybe I went too much ahead and maybe there are other ideas on implementing
> the change (or even maybe not implementing it at all).
>
> More context:
>
> I have just attempted what I THOUGHT was generally accepted and good idea,
> but comment from TP made me think maybe I went ahead of everyone else, so I
> am reaching here to see if this was a good idea (And I am ready to back-out
> and do it differently - and follow another way if someone can propose it).
>
> I just (literally while watching Poland <-> Brazil Volleybal World
> Championship semi-finals) prepared 48 separate PRs to migrate to
> __future__.annotations which (I thought) was considered a good idea and
> something we wanted to defer to after v2-4-test branch cut-off because of
> the concerns that it might make cherry-picking more complex. It was 9X%
> fully automated conversion - by search/replace and adding `from __future__
> import annotations` and letting upgrade do the job (as discussed before).
> I also split the one huge PR into 48 (!) much smaller PRs - basically one
> per each "airflow" core package, few big providers separately, tests
> separately (as they are far less risky than "core code" change).
>
> We already added __future__ annotation in a number of other places before
> - without a "broad" agreement and consensus/voting (different people did in
> various places - including myself and others) so I thought this is not
> something we need a broader voting/consensus on.
>
> Again - that was 95% automated change where I contributed mostly the
> thinking "how to split '' theI PR and all the rest was literally "git
> commit add <package>" (and let pyupgrade and our other pre-commits like
> isort and others) do the job.
>
> It never occured to me that this might be a problem for anyone. My
> understanding of the problem was:
> * we want to do it
> * we were afraid to do it before because it would make cherry-picking to
> 2-3 branch more problematic
> * we wanted to do it right "when" we cut-off the 2.4 branch (for the same
> reason as above - we wanted to avoid cherry-picking problems for 2.4.*
> changes)
>
> So I wanted to make this change as fast as humanly possible - and raise
> this multiple PRs so we can review/approve (and I'd cherry-pick them) as
> soon as possible to minimise the cherry-picking problem.
>
> But maybe I was too fast and it's not as straightforward as I thought it
> was? (actually it never occured to me that there might be a slightest
> problem with it after the earlier discussion). I literally thought about it
> as a mechanical change that we want to introduce and that doing that in
> small chunks (as discussed in the original PR) was the best approach. The
> comment from TP
> https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
> made me think that I have misjudged it.
>
> I wanted to write that email asking all of the committers here to help to
> review and merge all those PRs as fast as possible (again - those are
> purely mechanical changes, there are literally a few - literally less than
> 5) manual changes I had to make to fix some mypy problems.
> All the PRs are here:
> https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations
>
> But then I ask for something else then:
>
> * do you think this is fine we add __future__ annotations in general
> * do you think the way I proposed it is somewhat too In-human (i honestly
> feel it is an in-human change - because it is fully automated by our tools
> :D). But is it wrong approach to have 48 PRs automatically prepared by our
> tools to get a "consistent" approach
> * did I do it too fast/too early ?
> * any ideas on how we can do it differently to avoid the "cherry-picking"
> problems (mentioned by others in the original message)?
>
> I am really concerned to ask if this is something I have done wrongly or
> misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
> approach it differently and ask for consensus on such a flurry of PRS? Or
> maybe the timing is wrong.
>
> I am happy to follow in any way there - I can either keep the PRs and
> rebase them until the time is good (it takes about two minutes to rebase
> every one of them and apply the automation again)  if the way I proposed it
> was because I misunderstood the "sentiment" about the change - or follow
> any other way that will help us to implement the change - or even abandon
> it completely if we will agree it's not a good idea. I am also happy to
> apply any "general comments" there - now that the PR's are there, applying
> any automation even on all of them to improve it, is rather simple.
>
> Looking forward to any comments. I am really curious how others look at it
> - regardless of the outcome - always eager to learn new things :)
>
> J.
>
>
>
>
>
>
>
>
>
>
>
>
>
>
> On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
> tobiaszkedzierski@gmail.com> wrote:
>
>> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
>> Pierre for great isort tip :)
>>
>> BR
>> Tobiasz
>>
>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

Hey Everyone,

TL;DR; I planned to ask for help in reviewing the 48 (pretty much
fully automated PRs I prepared, but a comment from TP made me think that
maybe I went too much ahead and maybe there are other ideas on implementing
the change (or even maybe not implementing it at all).

More context:

I have just attempted what I THOUGHT was generally accepted and good idea,
but comment from TP made me think maybe I went ahead of everyone else, so I
am reaching here to see if this was a good idea (And I am ready to back-out
and do it differently - and follow another way if someone can propose it).

I just (literally while watching Poland <-> Brazil Volleybal World
Championship semi-finals) prepared 48 separate PRs to migrate to
__future__.annotations which (I thought) was considered a good idea and
something we wanted to defer to after v2-4-test branch cut-off because of
the concerns that it might make cherry-picking more complex. It was 9X%
fully automated conversion - by search/replace and adding `from __future__
import annotations` and letting upgrade do the job (as discussed before).
I also split the one huge PR into 48 (!) much smaller PRs - basically one
per each "airflow" core package, few big providers separately, tests
separately (as they are far less risky than "core code" change).

We already added __future__ annotation in a number of other places before -
without a "broad" agreement and consensus/voting (different people did in
various places - including myself and others) so I thought this is not
something we need a broader voting/consensus on.

Again - that was 95% automated change where I contributed mostly the
thinking "how to split '' theI PR and all the rest was literally "git
commit add <package>" (and let pyupgrade and our other pre-commits like
isort and others) do the job.

It never occured to me that this might be a problem for anyone. My
understanding of the problem was:
* we want to do it
* we were afraid to do it before because it would make cherry-picking to
2-3 branch more problematic
* we wanted to do it right "when" we cut-off the 2.4 branch (for the same
reason as above - we wanted to avoid cherry-picking problems for 2.4.*
changes)

So I wanted to make this change as fast as humanly possible - and raise
this multiple PRs so we can review/approve (and I'd cherry-pick them) as
soon as possible to minimise the cherry-picking problem.

But maybe I was too fast and it's not as straightforward as I thought it
was? (actually it never occured to me that there might be a slightest
problem with it after the earlier discussion). I literally thought about it
as a mechanical change that we want to introduce and that doing that in
small chunks (as discussed in the original PR) was the best approach. The
comment from TP
https://apache-airflow.slack.com/archives/CCPRP7943/p1662837864413849  -
made me think that I have misjudged it.

I wanted to write that email asking all of the committers here to help to
review and merge all those PRs as fast as possible (again - those are
purely mechanical changes, there are literally a few - literally less than
5) manual changes I had to make to fix some mypy problems.
All the PRs are here:
https://github.com/apache/airflow/pulls?q=is%3Aopen+is%3Apr+label%3Afuture-annotations

But then I ask for something else then:

* do you think this is fine we add __future__ annotations in general
* do you think the way I proposed it is somewhat too In-human (i honestly
feel it is an in-human change - because it is fully automated by our tools
:D). But is it wrong approach to have 48 PRs automatically prepared by our
tools to get a "consistent" approach
* did I do it too fast/too early ?
* any ideas on how we can do it differently to avoid the "cherry-picking"
problems (mentioned by others in the original message)?

I am really concerned to ask if this is something I have done wrongly or
misunderstood the consensus, and maybe chose the wrong way ? Maybe I should
approach it differently and ask for consensus on such a flurry of PRS? Or
maybe the timing is wrong.

I am happy to follow in any way there - I can either keep the PRs and
rebase them until the time is good (it takes about two minutes to rebase
every one of them and apply the automation again)  if the way I proposed it
was because I misunderstood the "sentiment" about the change - or follow
any other way that will help us to implement the change - or even abandon
it completely if we will agree it's not a good idea. I am also happy to
apply any "general comments" there - now that the PR's are there, applying
any automation even on all of them to improve it, is rather simple.

Looking forward to any comments. I am really curious how others look at it
- regardless of the outcome - always eager to learn new things :)

J.














On Thu, Jul 7, 2022 at 8:52 AM Tobiasz Kędzierski <
tobiaszkedzierski@gmail.com> wrote:

> Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
> Pierre for great isort tip :)
>
> BR
> Tobiasz
>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Tobiasz Kędzierski <to...@gmail.com>]

Great idea.  +1 from me. Thanks for raising this up Jarek and thanks to
Pierre for great isort tip :)

BR
Tobiasz

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jed Cunningham <je...@apache.org>]

Overall this sounds good to me. I'll echo waiting until after the last
planned 2.3 release. It'd be great if we could keep the empty inits empty
(and it sounds like isort will do that for us .

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jeambrun Pierre <pi...@gmail.com>]

Good to hear :)

Le mar. 5 juil. 2022 à 21:26, Ash Berlin-Taylor <as...@apache.org> a écrit :

> Oh And it works *really* nicely: It places it in the right order (very
> first import) and it doesn't add it to "empty" __init__.py files. Just what
> we want.
>
> On Tue, Jul 5 2022 at 20:22:48 +0100, Ash Berlin-Taylor <as...@apache.org>
> wrote:
>
> Oh not quite -- it will remove any unused imports, but I guess Pierre was
> suggesting using isort to add the future annotation line. Gotcha.
>
> -a
>
> On Tue, Jul 5 2022 at 20:21:14 +0100, Ash Berlin-Taylor <as...@apache.org>
> wrote:
>
> We've already got this in our pre-commit config that does the same (at
> least from what I can tell from a quick test)
>
> ```
>       - id: static-check-autoflake
>         name: Remove all unused code
>         entry: autoflake --remove-all-unused-imports
> --ignore-init-module-imports --in-place
>         language: python
>         additional_dependencies: ['autoflake']
> ```
>
> Added by one Jarek Potiuk https://github.com/apache/airflow/pull/20466 :D
>
>
> On Tue, Jul 5 2022 at 19:03:45 +0200, Jarek Potiuk <po...@apache.org>
> wrote:
>
> O wow. nice feature of isort :) TIL.
>
> On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre <pi...@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 <po...@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 <as...@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.
>>>>
>>>>
>>>>
>>>>
>>>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

Oh And it works /really/ nicely: It places it in the right order (very 
first import) and it doesn't add it to "empty" __init__.py files. Just 
what we want.

On Tue, Jul 5 2022 at 20:22:48 +0100, Ash Berlin-Taylor 
<as...@apache.org> wrote:
> Oh not quite -- it will remove any unused imports, but I guess Pierre 
> was suggesting using isort to add the future annotation line. Gotcha.
> 
> -a
> 
> On Tue, Jul 5 2022 at 20:21:14 +0100, Ash Berlin-Taylor 
> <as...@apache.org> wrote:
>> We've already got this in our pre-commit config that does the same 
>> (at least from what I can tell from a quick test)
>> 
>> ```
>>       - id: static-check-autoflake
>>         name: Remove all unused code
>>         entry: autoflake --remove-all-unused-imports 
>> --ignore-init-module-imports --in-place
>>         language: python
>>         additional_dependencies: ['autoflake']
>> ```
>> 
>> Added by one Jarek Potiuk 
>> <https://github.com/apache/airflow/pull/20466> :D
>> 
>> 
>> On Tue, Jul 5 2022 at 19:03:45 +0200, Jarek Potiuk 
>> <po...@apache.org> wrote:
>>> O wow. nice feature of isort :) TIL.
>>> 
>>> On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre 
>>> <pierrejbrun@gmail.com <ma...@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 <potiuk@apache.org 
>>>> <ma...@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 <ash@apache.org 
>>>>> <ma...@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 
>>>>>> <jarek@potiuk.com <ma...@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.
>>>>>>> 
>>>>>>> 
>>>>>>> 

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

Oh not quite -- it will remove any unused imports, but I guess Pierre 
was suggesting using isort to add the future annotation line. Gotcha.

-a

On Tue, Jul 5 2022 at 20:21:14 +0100, Ash Berlin-Taylor 
<as...@apache.org> wrote:
> We've already got this in our pre-commit config that does the same 
> (at least from what I can tell from a quick test)
> 
> ```
>       - id: static-check-autoflake
>         name: Remove all unused code
>         entry: autoflake --remove-all-unused-imports 
> --ignore-init-module-imports --in-place
>         language: python
>         additional_dependencies: ['autoflake']
> ```
> 
> Added by one Jarek Potiuk 
> <https://github.com/apache/airflow/pull/20466> :D
> 
> 
> On Tue, Jul 5 2022 at 19:03:45 +0200, Jarek Potiuk 
> <po...@apache.org> wrote:
>> O wow. nice feature of isort :) TIL.
>> 
>> On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre 
>> <pierrejbrun@gmail.com <ma...@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 <potiuk@apache.org 
>>> <ma...@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 <ash@apache.org 
>>>> <ma...@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 
>>>>> <jarek@potiuk.com <ma...@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.
>>>>>> 
>>>>>> 
>>>>>> 

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

We've already got this in our pre-commit config that does the same (at 
least from what I can tell from a quick test)

```
      - id: static-check-autoflake
        name: Remove all unused code
        entry: autoflake --remove-all-unused-imports 
--ignore-init-module-imports --in-place
        language: python
        additional_dependencies: ['autoflake']
```

Added by one Jarek Potiuk 
<https://github.com/apache/airflow/pull/20466> :D


On Tue, Jul 5 2022 at 19:03:45 +0200, Jarek Potiuk <po...@apache.org> 
wrote:
> O wow. nice feature of isort :) TIL.
> 
> On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre <pierrejbrun@gmail.com 
> <ma...@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 <potiuk@apache.org 
>> <ma...@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 <ash@apache.org 
>>> <ma...@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 
>>>> <jarek@potiuk.com <ma...@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.
>>>>> 
>>>>> 
>>>>> 

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

O wow. nice feature of isort :) TIL.

On Tue, Jul 5, 2022 at 7:01 PM Jeambrun Pierre <pi...@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 <po...@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 <as...@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.
>>>
>>>
>>>
>>>
>>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jeambrun Pierre <pi...@gmail.com>]

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 <po...@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 <as...@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.
>>
>>
>>
>>
>>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Jarek Potiuk <po...@apache.org>]

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 <as...@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.
>
>
>
>
>

Re: [DISCUSS] Migrate airflow code to use PEP 563 (postponed evaluation of annotations) MAYBE?

[Ash Berlin-Taylor <as...@apache.org>]

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.
> 
> 
>