mypy icon indicating copy to clipboard operation
mypy copied to clipboard

Implement flag to allow typechecking of untyped modules

Open DeinAlptraum opened this issue 1 year ago • 7 comments

Add a flag and config ini options "enable_installed_packages". Setting it to True instructs mypy to typecheck also modules that do not have stubs or a py.typed marker.

Fixes #8545

DeinAlptraum avatar Aug 27 '24 11:08 DeinAlptraum

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Aug 27 '24 12:08 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Aug 27 '24 12:08 github-actions[bot]

@hauntsaninja can you tell me by any chance if there's anything I'm missing/I should do to get a review here?

DeinAlptraum avatar Sep 09 '24 20:09 DeinAlptraum

Ping. Can someone review this? @JelleZijlstra can you help me with this?

DeinAlptraum avatar Sep 25 '24 10:09 DeinAlptraum

Ping. @hauntsaninja @JelleZijlstra can you tell me what I need to do to get a review here?

DeinAlptraum avatar Oct 02 '24 16:10 DeinAlptraum

Ping @hauntsaninja @JelleZijlstra @JukkaL

DeinAlptraum avatar Oct 13 '24 18:10 DeinAlptraum

Mypy development is unfortunately under-resourced and personally I don't have much time available to review PRs. I'd advise you to wait and hopefully a committer will come along and review this.

JelleZijlstra avatar Oct 13 '24 21:10 JelleZijlstra

I feel like this feature is crucial and affects way too many people for it to be ignored. Using type: ignore or manually adding a py.typed file to every used third-party module is incredibly impractical.

Could this at least get a CR, if improvements are needed then the community can follow up on issues for the PR. ❤️ @JukkaL @hauntsaninja

Xiddoc avatar Nov 22 '24 12:11 Xiddoc

I can try to review this -- possibly next week. It would help if the merge conflict was fixed.

JukkaL avatar Nov 22 '24 16:11 JukkaL

Thanks for the comment, didn't notice the merge conflict. I wish there were notifications for this... anyway, fixed.

DeinAlptraum avatar Nov 22 '24 18:11 DeinAlptraum

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 22 '24 18:11 github-actions[bot]

@hauntsaninja thanks for the feedback! You're right, this wasn't handling globs properly. In the process of fixing that, I also noticed that my solution was more complicated than necessary and simplified it a bit...

Regarding the name: it seems there was quite a lot of discussion about how the config option should be called on the original issue, so I wasn't sure what to pick. Yours definitely sounds better though, so changed it to that for now.

DeinAlptraum avatar Nov 23 '24 08:11 DeinAlptraum

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 23 '24 09:11 github-actions[bot]

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Nov 23 '24 21:11 github-actions[bot]

@hauntsaninja I added it to the docs where it felt sensible to me. Do you think it should be mentioned in any other places?

DeinAlptraum avatar Dec 02 '24 17:12 DeinAlptraum

According to mypy_primer, this change doesn't affect type check results on a corpus of open source code. ✅

github-actions[bot] avatar Dec 02 '24 18:12 github-actions[bot]

I tried this flag for Home Assistant today. Unfortunately, mypy couldn't even do a full run and failed instantly. Haven't narrowed it down completely yet, just a first reproducer (tested with Python 3.13):

# test.py
import aprslib.base91
reveal_type(1)
# mypy_config.ini
[mypy]
follow_imports = normal
follow_untyped_imports = true
pip install aprslib==0.7.2
mypy --config-file mypy_config.ini test.py
/.../venv/lib/python3.13/site-packages/aprslib/parsing/common.py:218: SyntaxWarning: invalid escape sequence '\!'
  match = re.findall("^(.*)\!([\x21-\x7b])([\x20-\x7b]{2})\!(.*?)$", body)
venv/lib/python3.13/site-packages/aprslib/parsing/__init__.py: error:
    Source file found twice under different module names: "aprslib.parsing" and "aprslib.parsing.__init__"
Found 1 error in 1 file (errors prevented further checking)

The SyntaxWarning is an unrelated issue.

Will open a new issue, once I've narrowed it down a bit more.

cdce8p avatar Dec 05 '24 12:12 cdce8p

@cdce8p thanks for reporting, but that seems unrelated to this change. I just cloned the aprs-python repo and ran the latest mypy release (i.e. before I added this flag) in the root folder of the project and I get the same error. Couldn't really tell you what that means though, since I'm not super familiar with the inner workings of mypy

DeinAlptraum avatar Dec 05 '24 12:12 DeinAlptraum

@cdce8p thanks for reporting, but that seems unrelated to this change. I just cloned the aprs-python repo and ran the latest mypy release (i.e. before I added this flag) in the root folder of the project and I get the same error.

Indeed thanks for pointing it out! Thinking out loud for a moment: Would it make sense to add an explicit warning to the docs or am I just the only one who would run into it? With follow_untyped_imports = true it's much more likely that code will be checked which isn't designed for type checking so errors might occur more frequently.

cdce8p avatar Dec 05 '24 19:12 cdce8p

Possibly same thing as https://github.com/python/mypy/issues/9716 ?

hauntsaninja avatar Dec 05 '24 20:12 hauntsaninja

Yeah looks like that might be right: https://github.com/rossengeorgiev/aprs-python/blob/master/aprslib/parsing/thirdparty.py#L2

hauntsaninja avatar Dec 05 '24 20:12 hauntsaninja

We can add a warning to the docs, done so here: https://github.com/python/mypy/pull/18249

hauntsaninja avatar Dec 05 '24 21:12 hauntsaninja