Add option to selectively disable --disallow-untyped-calls by ilevkivskyi · Pull Request #15845 · python/mypy (original) (raw)

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Conversation23 Commits4 Checks0 Files changed

Conversation

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

[ Show hidden characters]({{ revealButtonHref }})

Fixes #10757

It is surprisingly one of the most upvoted issues. Also it looks quite easy to implement, so why not. Note I also try to improve docs for per-module logic for disallow_untyped_calls, as there is currently some confusion.

This comment has been minimized.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this seems useful! Not a full review.


.. code-block:: python

# mypy --disallow-untyped-calls --untyped-call-exception=foo --untyped-call-exception=bar.A

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe also use a real third-party package in the example, such as numpy?

assert object_type is not None
fullname = self.method_fullname(object_type, member)
if not fullname or not any(
fullname.startswith(p) for p in self.chk.options.untyped_call_exception

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Make sure the exception re doesn't apply to retry, i.e. the startswith operation should probably be at module name component level.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good catch!

disallow_untyped_calls = True
untyped_call_exception = some.library

.. confval:: untyped_call_exception

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This seems useful, but I'm not sure about the name of the option. Maybe we can come up with a better name? I don't any suggestions right now, but I'll think about this later.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I also struggled some time with this. I would propose to not spend much time on this. People who want this will probably search for "mypy untyped call" or similar, so including these words in the option name should be good enough. So if you have a better idea, then I will be happy to update, otherwise let's just move on.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fwiw I'd go with untyped_call_allowlist. Clearly conveys the type as well ;-)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is what I considered at first, but then how wold we call corresponding command line option? --untyped-call-allowlist? All other options like --always-true, --always-false, --enable-error-code, --disable-error-code etc, are singular and should be repeated. So I would say we should try to follow this pattern. Finally, from reading comments in the issue, it looks like most people will need just 1 or maybe 2 exceptions, not more.

:type: comma-separated list of strings

Selectively excludes functions and methods defined in specific packages,
modules, and classes from action of :confval:`disallow_untyped_calls`.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mention that this also applies to submodules of packages (i.e. everything inside that prefix).

This flag allows to selectively disable :option:`--disallow-untyped-calls`
for functions and methods defined in specific packages, modules, or classes.
Note that each exception entry acts as a prefix. For example (assuming there
are no type annotations for ``numpy`` available):

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Numpy's probably not a great example here, as they ship with a py.typed file these days and have pretty complete type annotations. Maybe one of matplotlib, requests or TensorFlow might be a better example? Third-party stubs exist for all three packages, but none of them ships with a py.typed file

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any example of a popular library might become a bad example over time, maybe limit ourselves to something generic like third_party_lib?

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, let's use some generic name.

This comment has been minimized.

IIUC the only remaining question now is the name of the new flag. I propose that instead of keeping this open indefinitely, merge this as is, and then think about it until the next release. It will be easy to rename it if a better idea appears. If there are no objections, I am going to merge this tomorrow or Monday.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks, this is a nice feature. Agreed we can merge. How do you feel about just --untyped-call-allow? I like using "allow" because it's a verb we use in a lot of places and because exception makes me think of the language feature

Yes, -exception can make one think this is something about exceptions. But also then --untyped-call-allow would be too similar to --allow-untyped-calls (which is also a valid flag). I think it may be better to use --untyped-calls-exclude because:

It is also a verb an indeed we mostly use verbs for many flags (especially for strictness flags)
It doesn't have the exception connotation
It is sufficiently different from --exclude flag (and they are kind of similar btw, exclude also expects list of patterns)
It still seems to clearly indicate what it does: exclude certain functions from untyped calls check.

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

Hey, I just want to say, god bless you guys for adding this feature. I started using mypy last week and have thought that there has to be a way to do what untyped_calls_exclude does. It's hard to believe that this was only added this recently.

For those looking for a pyproject.toml config example for this:

[tool.mypy] strict = true untyped_calls_exclude = [ "skimage" ]

This ignores all errors from missing type annotations in the skimage third party package when imported and used in user code.