Skip to content

Cleanup MyPy Ignores #53241

Closed
Closed
Cleanup MyPy Ignores#53241
Labels
area:dev-envCI, pre-commit, pylint and other changes that do not change the behavior of the final codekind:metaHigh-level information important to the community

Description

@gopidesupavan
gopidesupavan
opened on Jul 12, 2025

Body

We have migrated mypy to 1.16.1, now we can cleanup type ignores where possible.

Follow the below steps

  1. Build lates ci image with breeze build ci-imgae --python 3.10
  2. export SHOW_UNUSED_MYPY_WARNINGS=true
  3. Run the ./scripts/ci/pre_commit/mypy_folder.py {Provider Name}

This will show what are all possible type ignores we can remove.

An example output looks like this.

Image

Remove all the unused type ignores and run it again see if anything breaks

Raise a PR with title Cleanup type ignores in <provider> provider where possible

ref PR: #53239

Committer

  • I acknowledge that I am a maintainer/committer of the Apache Airflow project.

Activity

  1. gopidesupavan
    added
    kind:metaHigh-level information important to the community
    on Jul 12, 2025
  2. dosubot
    added
    area:dev-envCI, pre-commit, pylint and other changes that do not change the behavior of the final code
    on Jul 12, 2025
  3. gopidesupavan
    changed the title [-][NOT READY] Cleanup MyPy Ignores[/-] [+]Cleanup MyPy Ignores[/+] on Jul 12, 2025

  4. potiuk commented on Jul 12, 2025

    @potiuk
    potiuk
    on Jul 12, 2025
    Member

    👍

  5. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    Some parts of the code still produce mypy errors even after my changes. Is it okay to leave them as is, or should I fix those as well?

  6. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    @mingdaoy I noticed there’s some overlap between our work. I’ll cherry-pick your changes and clean up the duplicated parts on my side. Thanks for working on it first!

  7. mingdaoy commented on Jul 13, 2025

    @mingdaoy
    mingdaoy
    on Jul 13, 2025
    Contributor

    @kyungjunleeme I had apache folder PR already. What are you trying to work on?

  8. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    @mingdaoy
    There are many providers under the apache folder, so I’ll focus on ones you didn’t include in your PR — specifically pinot and tinkerpop, if that works for you.

  9. mingdaoy commented on Jul 13, 2025

    @mingdaoy
    mingdaoy
    on Jul 13, 2025
    Contributor

    @kyungjunleeme Sure. Go ahead. Did you follow the steps described in this issue? I checked and it seems that nothing needs to be removed in pinot

  10. gopidesupavan commented on Jul 13, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 13, 2025
    MemberAuthor

    Some parts of the code still produce mypy errors even after my changes. Is it okay to leave them as is, or should I fix those as well?

    no, if you see any errors apart from the type unused please put it back those ignores :)

  11. gopidesupavan commented on Jul 13, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 13, 2025
    MemberAuthor

    I have updated with apache providers list please pickup individual providers

  12. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    @gopidesupavan Yes, fortunately, I worked on it that way.

    @mingdaoy

    Thanks to you, I was able to finish my work as well.
    Would you mind taking a look at my PR?

  13. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    @mingdaoy um. yes. I will re-check right away. just a moment

  14. mingdaoy commented on Jul 13, 2025

    @mingdaoy
    mingdaoy
    on Jul 13, 2025
    Contributor

    @kyungjunleeme I checked the rest of the Apache folder as well and it seems "no issues found". You can check again. I might be wrong. cc. @gopidesupavan

    ./scripts/ci/pre_commit/mypy_folder.py providers/apache/pinot
    Image 
    ./scripts/ci/pre_commit/mypy_folder.py providers/apache/tinkerpop
    Image

  15. mingdaoy commented on Jul 13, 2025

    @mingdaoy
    mingdaoy
    on Jul 13, 2025
    Contributor

    @gopidesupavan I checked the rest of all the unassigned and unchecked providers. I got "Success: no issues found in X source files"

    I checked three other providers in common as well.

    ./scripts/ci/pre_commit/mypy_folder.py providers/common/compat
./scripts/ci/pre_commit/mypy_folder.py providers/common/sql
./scripts/ci/pre_commit/mypy_folder.py providers/common/messaging

  16. 7 remaining items

  17. gopidesupavan commented on Jul 13, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 13, 2025
    MemberAuthor

    oh these are coming only when we run full providers , if we are running individual providers these are getting suppressed.

  18. kyungjunleeme commented on Jul 13, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 13, 2025
    Contributor

    um...

    I think that this result caused by 1) scripts/ci/pre_commit/mypy_folder.py
    and 2) mypy internal judgement logic issue.
    Um. Its only my guess. If I find exactly cause. Share it.

  19. kyungjunleeme commented on Jul 14, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 14, 2025
    Contributor

    @gopidesupavan Colud I have a question about clenaup check process?

    Follow the below steps

    1. Build lates ci image with breeze ci-imgae build --python 3.10 (typo. I fixed it)
    2. export SHOW_UNUSED_MYPY_WARNINGS=true
    3. Run the ./scripts/ci/pre_commit/mypy_folder.py {Provider Name}
      This will show what are all possible type ignores we can remove.

    I think that, between 2 and 3, 3. breeze shell python=3.10 --mount-sources all this command shoud be added? am I right?
    actually, Im not familiar with breeze development. plz understand quality of my question

  20. gopidesupavan commented on Jul 14, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 14, 2025
    MemberAuthor

    hey @kyungjunleeme no those commands should execute outside of breeze, the mypy runs inside breeze, when you run scripts/ci/pre_commit/mypy_folder.py this spins up breeze and executes all the mypy checks, so basically you no need to go inside breeze shell.

    if your interested running mypy inside breeze shell manually . you can do like this.
    eg:

    breeze shell
mypy providers/google/src/airflow/providers/google/firebase/operators/firestore.py --warn-unused-ignores

  21. kyungjunleeme commented on Jul 14, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 14, 2025
    Contributor

    Thank you — I wasn’t aware of that. I’ll take this opportunity to dig a bit deeper into how mypy is used in Airflow.

    As for #53330, it might not actually be a bug after all…

  22. gopidesupavan commented on Jul 14, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 14, 2025
    MemberAuthor

    Thank you — I wasn’t aware of that. I’ll take this opportunity to dig a bit deeper into how mypy is used in Airflow.

    As for #53330, it might not actually be a bug after all…

    cool, i did some investigation,

    I think mypy getting confused with module namespace, eg: google provider, in airflow we have google.cloud, but gcs cloudstorage library also has from google.cloud import storage, so mypy might treating this is not available in airflow repo google.cloud , will workout something later.

  23. kyungjunleeme commented on Jul 14, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 14, 2025
    Contributor

    FYI

    I got help from ai. but. I have checked all link and about contents.
    it can be helpful to have a intresting about type hinting.

    🧩 Namespace Resolution Issues Across Type Checkers

    🔎 These issues all relate to PEP 420, which introduced namespace packages—packages split across multiple directories.
    Tools like mypy and pyright often struggle with correctly resolving these when both user code and installed libraries share the same namespace (e.g. google.cloud).

    Tool / Source Issue Link Summary Relevance to Airflow
    PEP 420 PEP 420 – Implicit Namespace Packages Introduces namespace packages that allow splitting a package across multiple directories. Airflow's providers/google/cloud/... overlaps with google-cloud-* libraries in site-packages.
    mypy #10360 Fails to resolve google.cloud.logging due to namespace conflicts; only the first path is used. Airflow's providers/google/cloud may shadow installed google-cloud-* modules.
    mypy #12985 Reports missing attributes like google.cloud.firestore due to resolution issues or missing stubs. Affects modules using third-party Google libraries.
    pyright #2882 Cannot distinguish between user-defined and installed google.cloud modules. Impacts Airflow when pyright is used in monorepos or complex provider structures.
    StackOverflow #75835248 mypy fails to detect type mismatches due to misinterpreted namespace packages. Matches Airflow’s namespace layout where overlapping paths confuse the type checker.

  24. gopidesupavan commented on Jul 14, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 14, 2025
    MemberAuthor

    FYI

    I got help from ai. but. I have checked all link and about contents. it can be helpful to have a intresting about type hinting.

    🧩 Namespace Resolution Issues Across Type Checkers

    🔎 These issues all relate to PEP 420, which introduced namespace packages—packages split across multiple directories.
    Tools like mypy and pyright often struggle with correctly resolving these when both user code and installed libraries share the same namespace (e.g. google.cloud).

    Tool / Source Issue Link Summary Relevance to Airflow
    PEP 420 PEP 420 – Implicit Namespace Packages Introduces namespace packages that allow splitting a package across multiple directories. Airflow's providers/google/cloud/... overlaps with google-cloud-* libraries in site-packages.
    mypy #10360 Fails to resolve google.cloud.logging due to namespace conflicts; only the first path is used. Airflow's providers/google/cloud may shadow installed google-cloud-* modules.
    mypy #12985 Reports missing attributes like google.cloud.firestore due to resolution issues or missing stubs. Affects modules using third-party Google libraries.
    pyright #2882 Cannot distinguish between user-defined and installed google.cloud modules. Impacts Airflow when pyright is used in monorepos or complex provider structures.
    StackOverflow #75835248 mypy fails to detect type mismatches due to misinterpreted namespace packages. Matches Airflow’s namespace layout where overlapping paths confuse the type checker.

    yes exactly it conflicts the namespaces :) , have modified the imports to use full path #53338. will see probably we can go with alias technique thats the simplest one.

  25. mingdaoy commented on Jul 14, 2025

    @mingdaoy
    mingdaoy
    on Jul 14, 2025
    Contributor

    #53241 (comment)

    @kyungjunleeme When I ran ./scripts/ci/pre_commit/mypy_folder.py providers/apache/pinot and tinkerpop , it showed "0 issues found". However, your PR #53311 actually removed some type ignores.
    Maybe step 3 does not show all possible type ignores we can remove.

  26. kyungjunleeme commented on Jul 15, 2025

    @kyungjunleeme
    kyungjunleeme
    on Jul 15, 2025
    Contributor

    @mingdaoy I guess that ur breeze development enviornment is something wrong

  27. mingdaoy commented on Jul 15, 2025

    @mingdaoy
    mingdaoy
    on Jul 15, 2025
    Contributor

    @gopidesupavan @jscheffl
    Do we need to run <step 1> breeze ci-image build --python 3.10 every time before running mypy checks on each provider ./scripts/ci/pre_commit/mypy_folder.py providers/{NAME}? Building the image can take a while.

  28. gopidesupavan commented on Jul 15, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 15, 2025
    MemberAuthor

    @gopidesupavan @jscheffl Do we need to run <step 1> breeze ci-image build --python 3.10 every time before running mypy checks on each provider ./scripts/ci/pre_commit/mypy_folder.py providers/{NAME}? Building the image can take a while.

    not required image build is only onetime activity , if any deps changed you may need to build.

  29. gopidesupavan commented on Jul 15, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 15, 2025
    MemberAuthor

    #53241 (comment)

    @kyungjunleeme When I ran ./scripts/ci/pre_commit/mypy_folder.py providers/apache/pinot and tinkerpop , it showed "0 issues found". However, your PR #53311 actually removed some type ignores. Maybe step 3 does not show all possible type ignores we can remove.

    might be it related to cache, mypy cache what it runs, if you dont want to use cache when running mypy command i think you can add this --no-incremental

  30. gopidesupavan commented on Jul 15, 2025

    @gopidesupavan
    gopidesupavan
    on Jul 15, 2025
    MemberAuthor

    Thank you everyone this is completed now :), appreciate your time and effort.

  31. gopidesupavan
    closed this as completedon Jul 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    area:dev-envCI, pre-commit, pylint and other changes that do not change the behavior of the final codekind:metaHigh-level information important to the community

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions