Uh oh!
There was an error while loading. Please reload this page.
Cope with Pycharm's oddity in detecting namespace packages - #43116
Closed
ashb wants to merge 1 commit into
Closed
Conversation
Without the `__path__` assignment being the very first line of the file (including before any import) it doesn't detect this correctly.
ashb
commented
Oct 17, 2024
MemberAuthor
kaxil
commented
Oct 17, 2024
Member
kaxil
approved these changes
Oct 17, 2024
kaxil
reviewed
Oct 17, 2024
| # And it _has_ to be the first code line too. Sad panda | ||
| __path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore | ||
| from __future__ import annotations # noqa: F404 |
Member
There was a problem hiding this comment.
Although:
Python mandates that future-imports must appear in the module before any other code except docstrings
Contributor
There was a problem hiding this comment.
Question to the audience - can we live with it (or similar workaround) for now until we find a better alternative?
Partial answer - mypy and some static checks don't like it...
MemberAuthor
There was a problem hiding this comment.
This really does feel like a massive bug in pycharm :(
Member
There was a problem hiding this comment.
Yeah, for now I think we can live with that bug.
Once we move a good number of things from Airflow to SDK, maybe the airflow/__init__.py file won't have much content left to keep from __future__ import annotations!
shahar1
reviewed
Oct 17, 2024
| # Make `airflow` a namespace package, supporting installing | ||
| # airflow.providers.* in different locations (i.e. one in site, and one in user | ||
| # lib.) This is required by some IDEs to resolve the import paths. | ||
| # And it _has_ to be the first code line too. Sad panda |
| # And it _has_ to be the first code line too. Sad panda | ||
| __path__ = __import__("pkgutil").extend_path(__path__, __name__) # type: ignore | ||
| from __future__ import annotations # noqa: F404 |
Contributor
There was a problem hiding this comment.
Question to the audience - can we live with it (or similar workaround) for now until we find a better alternative?
Partial answer - mypy and some static checks don't like it...
vincbeck
commented
Oct 17, 2024
Contributor
Yeah I tried in #43081 but could not make it work. Mypy/Python is not happy about it and raise a |
ashb
commented
Oct 17, 2024
MemberAuthor
https://youtrack.jetbrains.com/issue/PY-46690/Allow-all-imports-from-the-same-named-packages-if-path-manipulation-in-init-is-detected is the closest current issue I've found, but not exactly the same |
Member
Let's just exclude We've done that for Pydantic classes before and still doing it for some test files https://github.com/apache/airflow/blob/main/pyproject.toml#L355 |
potiuk added a commit
to potiuk/airflow
that referenced
this pull request
Oct 18, 2024
Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. Also removed the `__init__.py` file from "providers" directory, it is not needed there, because "providers" is just a folder where we keep provider files, it's not a Python package. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccesful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code.
potiuk
commented
Oct 18, 2024
Member
I added alternative implementation - and it seems to work for me #43173 - just by removing |
potiuk added a commit
to potiuk/airflow
that referenced
this pull request
Oct 18, 2024
Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. Also removed the `__init__.py` file from "providers" directory, it is not needed there, because "providers" is just a folder where we keep provider files, it's not a Python package. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccesful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes a few other teething problems with setup of tests that were introcuded in apache#42505 and apache#43802 "masked" by having `__init__.py` added in providers package: * openlineage extractor test that should not expect "providers.tests.*" but "tests.*" package * common_sql_api_stubs wrongly calculating generated path for stub-generated files * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present)
potiuk added a commit
to potiuk/airflow
that referenced
this pull request
Oct 18, 2024
Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. Also removed the `__init__.py` file from "providers" directory, it is not needed there, because "providers" is just a folder where we keep provider files, it's not a Python package. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccesful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes a few other teething problems with setup of tests that were introcuded in apache#42505 and apache#43802 "masked" by having `__init__.py` added in providers package: * common.sql interface pre-commit used wrong path to generated files * openlineage extractor test that should not expect "providers.tests.*" but "tests.*" package * common_sql_api_stubs wrongly calculating generated path for stub-generated files * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present)
kaxil pushed a commit
to potiuk/airflow
that referenced
this pull request
Oct 18, 2024
Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. Also removed the `__init__.py` file from "providers" directory, it is not needed there, because "providers" is just a folder where we keep provider files, it's not a Python package. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccesful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes a few other teething problems with setup of tests that were introcuded in apache#42505 and apache#43802 "masked" by having `__init__.py` added in providers package: * common.sql interface pre-commit used wrong path to generated files * openlineage extractor test that should not expect "providers.tests.*" but "tests.*" package * common_sql_api_stubs wrongly calculating generated path for stub-generated files * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present)
kaxil added a commit
that referenced
this pull request
Oct 18, 2024
…3173) Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. That should finally get rid of the Intellij teething import problem that has been introduced in #42505. There were earlier - unsuccessful - attempts to fix it in the #43116 and #43081 that followed #42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes following problem: * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present) --------- Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
harjeevanmaan pushed a commit
to harjeevanmaan/airflow
that referenced
this pull request
Oct 23, 2024
…ache#43173) Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccessful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes following problem: * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present) --------- Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
PaulKobow7536 pushed a commit
to PaulKobow7536/airflow
that referenced
this pull request
Oct 24, 2024
…ache#43173) Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccessful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes following problem: * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present) --------- Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
potiuk
commented
Oct 29, 2024
Member
Closing this one. It's already fixed. |
ellisms pushed a commit
to ellisms/airflow
that referenced
this pull request
Nov 13, 2024
…ache#43173) Cleans-up airflow and providers `__init__.py" files in order to get providers import work again. This is done by excluding the two `__init__.py` files from automated ruff isort rules adding `from __future__ import annotations`. That should finally get rid of the Intellij teething import problem that has been introduced in apache#42505. There were earlier - unsuccessful - attempts to fix it in the apache#43116 and apache#43081 that followed apache#42951 - but the key is that Pycharm requires the namespace's extend_path to be first "real" line of code in the `__init__.py` to understand that the package is an "explicit" namespace package - and it conflicts with the requirement of "from __future__ import annotations" to be the first line of Python code. Also this PR fixes following problem: * pytest_plugin expecting .asf.yml in "airflow" sources - even during compatibility tests with older version of airflow (where the .asf.yml is not present) --------- Co-authored-by: Kaxil Naik <kaxilnaik@gmail.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for freeto join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Without the
__path__assignment being the very first line of the file(including before any import) it doesn't detect this correctly.