Gah not only that, vscode also ignores __all__ settings which is really bad
form.
Pycharm at least does respect __all__ for what to show in the module, and also
respects type stub files (.pyi) when completing constructor args etc.
In short IDEs are terrible at dynamic languages, but it is what it is.
So a few options then:
Just have a deprecation warning (which it's not even clear if PEP-562 style
will show a nice (!) warning in IDEs or just at run time
I learn to live with from airflow.models.dag import DAG
After we change airflow.models to not import all the sub-modules automatically,
go back to importing "airflow.models.dag.DAG" into "airflow".
We create a new (abstract) base class that we _can_ import in to the airflow
module that airflow.models.dag.DAG extends. This could be automated by a
pre-commit so it is always up to date with the real DAG class
We create a new module of things that are part of the public/dag author API,
such as airflow.dag (currently just has a "BaseDAG" sub-module but that is
barely used, and only by the scheduler so could be removed)
Anyone have any strong opinions? I think I'd favour 3, 4 or 5
-ash
On Feb 22 2020, at 10:50 pm, Kaxil Naik <kaxiln...@gmail.com> wrote:
> I tried an example with PEP-562 but the autocomplete didn't work in Pycharm
> and still showed the deprecated function.
>
> I agree we should have a deprecation warning before we should have changed
> it. How about we introduce a deprecation warning in the next version
> (1.10.10) ?
>
> Regards,
> Kaxil
> ᐧ
>
> On Sat, Feb 22, 2020 at 5:35 PM Jarek Potiuk <jarek.pot...@polidea.com>
> wrote:
>
> > I also started to use VSCode in parallel on my Chromebook as this is the
> > best way to get devenv running there (and I believe it's now THE most
> > popular IDE - including for Python developers). I can check it there as
> > well.
> >
> > On Sat, Feb 22, 2020 at 6:33 PM Jarek Potiuk <jarek.pot...@polidea.com>
> > wrote:
> >
> > > Right -> the same. Happy to double check with your POC :)
> > > On Sat, Feb 22, 2020 at 6:16 PM Ash Berlin-Taylor <a...@apache.org>
> > wrote:
> > > >
> > > > That's a very good point about IDE's, I'll double check how PyCharm
> > > behaves (Guessing PyCharm is the most popular one? PyCharm and IntelliJ
> >
> > are
> > > the same engine under the hood, right?)
> > > >
> > > > -a
> > > > On Feb 22 2020, at 4:58 pm, Jarek Potiuk <jarek.pot...@polidea.com>
> > >
> > > wrote:
> > > > > > My proposal here:
> > > > > > Add in a _getattr_ based lazy import for DAG to airflow/__init__.py
> > > > >
> > > >
> > >
> > > module
> > > > >
> > > > >
> > > > > I am all for it - if we can do it in this way, then it is indeed
> > > > > better for the users.
> > > > >
> > > > > >
> > > > > > Do NOT issue a deprecation warning for this.
> > > > > > Revert the change to all the imports in example dags etc so that
> > > > >
> > > >
> > >
> > > those do from airflow import DAG.
> > > > > I would only be for it if the PEP-562 change works fine with IDE
> > > > > support. Many of our users use IDEs to write their DAGsand they can
> > > > > get confused where to import the DAGs from. If the PEP-562 works fine
> > > > > with the IDEs, and they are smart enough to figure this out, I am ok
> > > > > with that.
> > > > >
> > > > > On a related note - I fully agree with the "clear API" approach for
> > > > > DAG developers. We did not have clear rules for that so far.
> > > > > Eventually (I think 2.0 is good time for that) we should have a clear
> > > > > statement what really the API is for DAG writers. We should issue at
> > > > > first a deprecation warning and then error if someone tries to use
> > > > > something not explicitly allowed from the DAG. It should not be a
> > > > > convention ("from airflow.* is allowed in documentation"). It should
> > > > > be enforcement. We can do it in scheduler - as we are parsing the
> > > >
> > >
> >
> > DAGs
> > > > > on our own and we can find out what imports the users are using. We
> > > > > could even think about adding those deprecations in 2.0. There are
> > > >
> > >
> >
> > not
> > > > > that many things that should be importable from 'airflow.*'. And that
> > > > > should be rather easy - I am playing now with AST parser and it is
> > > > > rather straightforward - we just need to generate list of "allowed"
> > > > > imports .
> > > > >
> > > > > > (I'm less worried about the internal API airflow users internally,
> > > so airflow.models.TaskInstance vs
> >
> > airflow.models.task_instance.TaskInstance
> > > is okay for me. I find the later uglier but if it doesn't affect users I
> > > don't mind it)
> > > > > > Ash
> > > > >
> > > > > --
> > > > > Jarek Potiuk
> > > > > Polidea | Principal Software Engineer
> > > > >
> > > > > M: +48 660 796 129
> > >
> > >
> > > --
> > > Jarek Potiuk
> > > Polidea | Principal Software Engineer
> > >
> > > M: +48 660 796 129
> >
> >
> > --
> > Jarek Potiuk
> > Polidea <https://www.polidea.com/> | Principal Software Engineer
> >
> > M: +48 660 796 129 <+48660796129>
> > [image: Polidea] <https://www.polidea.com/>
>
>
