This issue tracker has been migrated to GitHub, and is currently read-only.
For more information, see the GitHub FAQs in the Python's Developer Guide.

classification
Title: typing: remove callable() check from typing._type_check
Type: enhancement Stage: resolved
Components: Library (Lib) Versions:
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: Nosy List: AlexWaygood, GBeauregard, Gobot1234, JelleZijlstra, kj, levkivskyi, serhiy.storchaka, sobolevn, uriyyo
Priority: normal Keywords: patch

Created on 2022-02-04 23:02 by GBeauregard, last changed 2022-04-11 14:59 by admin. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 31151 merged GBeauregard, 2022-02-05 22:27
PR 31159 closed serhiy.storchaka, 2022-02-06 12:01
PR 31833 merged JelleZijlstra, 2022-03-12 01:49
PR 27553 uriyyo, 2022-03-12 18:01
Messages (17)
msg412548 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-04 23:02
I propose removing the callable() check[1] from typing._type_check. This restriction is usually met in typeform instances by implementing a __call__ method that raises at runtime[2]. _type_check is called at runtime in a few disparate locations, such as in an argument to typing.Annotated or for certain stringified annotations in typing.get_type_hints.

Because the requirement to be callable is unexpected and shows up in situations not easily discoverable during development or common typing usage, it is the cause of several existing cpython bugs and will likely continue to be the cause of bugs in typeforms outside of cpython. Known cpython bugs caused by the callable() check are bpo-46643, bpo-44799, a substantial contributing factor to bpo-46642, and partly bpo-46511. I discovered bpo-46643 with only a cursory check of typing.py while writing this proposal. Moreover, it doesn't make any particular technical sense to me why it should be required to add an awkward __call__ method.

Removing the callable() check fails 10 tests:
7 tests: checking that an int literal is not a type
2 tests: testing that list literals are not valid types (e.g. [3] raises a TypeError because the literal [('name', str), ('id', int)] doesn't pass callable())
1 test: bpo-46642

The responsibility of determining these invalid typeforms (e.g. int literals) would need to be passed to a static type checker. If it's desired to do this at runtime it's my opinion that a different check would be more appropriate.

Have I missed any reasons for the callable() check? Can I remove the check and adjust or remove the tests?

[1] https://github.com/python/cpython/blob/bf95ff91f2c1fc5a57190491f9ccdc63458b089e/Lib/typing.py#L183-L184
[2] https://github.com/python/cpython/blob/bf95ff91f2c1fc5a57190491f9ccdc63458b089e/Lib/typing.py#L392-L393
[3] https://github.com/python/cpython/blob/bf95ff91f2c1fc5a57190491f9ccdc63458b089e/Lib/test/test_typing.py#L4262-L4263
msg412550 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-04 23:35
In addition to the 10 tests failed in test_typing.py, one additional test fails in test_types.py with this change: https://github.com/python/cpython/blob/bf95ff91f2c1fc5a57190491f9ccdc63458b089e/Lib/test/test_types.py#L834-L838 and those are all the tests in cpython changed.

This falls in category (1), checking that an int literal is not a type, but with the apparent intent to prevent index subscripting.
msg412584 - (view) Author: Gobot1234 (Gobot1234) * Date: 2022-02-05 17:43
I also support this change. I've had to write a lot of code to make SpecialForms able to accept my types where the code has to look like:
```py
class Something:
    ...
    def __call__(self, *args, **kwargs):
       raise NotImplementedError
```
I also know this comes up in typing-extensions a fair bit. I think type checkers should be enforcing this at type-checking-time not by typing.py run-time.
msg412585 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-02-05 17:43
I agree with removing this check. I suspect it's a holdover from very early typing when static types were supposed to be runtime types. Now the check is a bug magnet and doesn't serve a useful purpose.

I think we can just remove the tests that check for ints. I don't see a principled reason to special-case int literals.

I wonder if we should apply this change to 3.10 and 3.9. It's arguably a bugfix, but it's a pretty big change.
msg412596 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-05 20:59
Under the same failing int test cases before there were 2 more cases next to them that fail:

with self.assertRaises(TypeError):
    ClassVar[int, str]
with self.assertRaises(TypeError):
    Final[int, str]

These fail because tuple literals are not callable(). There is code that clearly intends for this to be the case: https://github.com/python/cpython/blob/96b344c2f15cb09251018f57f19643fe20637392/Lib/typing.py#L486

I can either remove support for this runtime check or change the implementation of Final et al to reject tuple literals. I will do the latter for now.

For https://github.com/python/cpython/blob/bf95ff91f2c1fc5a57190491f9ccdc63458b089e/Lib/test/test_typing.py#L4262-L4263 I think the best approach is to just remove these tests.
msg412597 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-05 21:23
Further questions: the msg argument in _type_check now wouldn't be used for anything! It was only used in the case where a type wasn't callable(). I think it should be removed. I'm also a bit negative on disallowing tuples in the case of e.g. Final and such since it complicates implementing tuple types in Python down the line if desired.
msg412599 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-05 22:08
I made a draft pull request where I went ahead and added a check to disallow tuple literals. This is basically already disallowed for types by restrictions on `__getitem__` because Union[typeform]->type needs to be different from Union[type,type]->Union[type,type].
msg412623 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-02-06 12:01
There were two reasons of accepting arbitrary callables in _type_check():

1. NewType() returned a function.
2. xml.etree.cElementTree.Element was a function in Python 2.

Now NewType is a class, and Python 2 no longer supported. I agree that we should get rid of the callable() check, but I disagree with the proposed change. The check should be more explicit and strict, not more lenient. List[42] does not make sense and should not be accepted.
msg412651 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-06 17:59
List[42] is already accepted, and your proposed patch does not change it to make it not accepted. The issue is _type_check is only called in a few particular locations; this is part of the technical reason I'm not very concerned about relaxing the _type_check requirements.

From a type checking philosophy point of view I agree with Jelle and am negative on strict runtime requirements in typing.py.
msg412657 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-02-06 19:39
No, List[42] is not currently accepted.

>>> List[42]
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/serhiy/py/cpython/Lib/typing.py", line 318, in inner
    return func(*args, **kwds)
           ^^^^^^^^^^^^^^^^^^^
  File "/home/serhiy/py/cpython/Lib/typing.py", line 1127, in __getitem__
    params = tuple(_type_check(p, msg) for p in params)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/home/serhiy/py/cpython/Lib/typing.py", line 1127, in <genexpr>
    params = tuple(_type_check(p, msg) for p in params)
                   ^^^^^^^^^^^^^^^^^^^
  File "/home/serhiy/py/cpython/Lib/typing.py", line 184, in _type_check
    raise TypeError(f"{msg} Got {arg!r:.100}.")
    ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
TypeError: Parameters to generic types must be types. Got 42.
msg412658 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-06 19:48
I'm referring to within type annotations, where this code path isn't used: try a: List[42]

This code path can show up in type aliases though.
msg412660 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2022-02-06 20:12
But it gives the same result.

What version of Python do you test with?
msg412662 - (view) Author: Gregory Beauregard (GBeauregard) * Date: 2022-02-06 20:28
I compiled your PR to run it and was testing in 3.10 as well, but I was testing in a file with from __future__ import annotations unintentionally. I retract the comment. It turns out `list[42]` is okay though, which I suppose is more relevant going forward. My confusion here is sort of the crux of my problem with these runtime checks: they are inconsistently applied in different locations which is why callable() was causing a lot of bugs.
msg414961 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-03-12 01:12
New changeset 870b22b9c442d035190d2b8fb82256cd9a03da48 by Gregory Beauregard in branch 'main':
bpo-46644: Remove callable() requirement from typing._type_check (GH-31151)
https://github.com/python/cpython/commit/870b22b9c442d035190d2b8fb82256cd9a03da48
msg414962 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-03-12 01:12
Thanks for your contribution!
msg414966 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-03-12 01:37
Some tests are failing on main, probably due to a race. PR incoming.
msg414967 - (view) Author: Jelle Zijlstra (JelleZijlstra) * (Python committer) Date: 2022-03-12 02:17
New changeset 75174371e6cac935b598a68c1113f6db1e0d6ed8 by Jelle Zijlstra in branch 'main':
bpo-46644: Fix test_typing test broken by GH-31151 due to a merge race (GH-31833)
https://github.com/python/cpython/commit/75174371e6cac935b598a68c1113f6db1e0d6ed8
History
Date User Action Args
2022-04-11 14:59:55adminsetgithub: 90802
2022-03-12 18:01:12uriyyosetnosy: + uriyyo

pull_requests: + pull_request29939
2022-03-12 02:46:05JelleZijlstrasetstatus: open -> closed
resolution: fixed
stage: patch review -> resolved
2022-03-12 02:17:53JelleZijlstrasetmessages: + msg414967
2022-03-12 01:49:57JelleZijlstrasetstage: resolved -> patch review
pull_requests: + pull_request29930
2022-03-12 01:37:30JelleZijlstrasetstatus: closed -> open
resolution: fixed -> (no value)
messages: + msg414966
2022-03-12 01:12:52JelleZijlstrasetstatus: open -> closed
resolution: fixed
messages: + msg414962

stage: patch review -> resolved
2022-03-12 01:12:25JelleZijlstrasetmessages: + msg414961
2022-02-06 23:05:41gvanrossumsetnosy: - gvanrossum
2022-02-06 20:28:55GBeauregardsetmessages: + msg412662
2022-02-06 20:12:51serhiy.storchakasetmessages: + msg412660
2022-02-06 19:48:33GBeauregardsetmessages: + msg412658
2022-02-06 19:39:53serhiy.storchakasetmessages: + msg412657
2022-02-06 17:59:09GBeauregardsetmessages: + msg412651
2022-02-06 12:01:20serhiy.storchakasetpull_requests: + pull_request29332
2022-02-06 12:01:12serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg412623
2022-02-05 22:27:29GBeauregardsetkeywords: + patch
stage: patch review
pull_requests: + pull_request29328
2022-02-05 22:08:08GBeauregardsetmessages: + msg412599
2022-02-05 21:23:10GBeauregardsetmessages: + msg412597
2022-02-05 20:59:57GBeauregardsetmessages: + msg412596
2022-02-05 17:43:27JelleZijlstrasetmessages: + msg412585
2022-02-05 17:43:13Gobot1234setnosy: + Gobot1234
messages: + msg412584
2022-02-04 23:35:42GBeauregardsetmessages: + msg412550
2022-02-04 23:02:57GBeauregardcreate