Skip to content

Make builtins.callable "generic" #86268

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 and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
hauntsaninja opened this issue Oct 20, 2020 · 15 comments
Open

Make builtins.callable "generic" #86268

hauntsaninja opened this issue Oct 20, 2020 · 15 comments
Labels
topic-typing type-feature A feature request or enhancement

Comments

@hauntsaninja
Copy link
Contributor

BPO 42102
Nosy @gvanrossum, @serhiy-storchaka, @ilevkivskyi, @isidentical, @hauntsaninja, @Fidget-Spinner
PRs
  • bpo-42102: [draft] make callable runtime subscriptable #22848
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2020-10-20.21:48:49.801>
    labels = ['type-feature', '3.10']
    title = 'Make builtins.callable "generic"'
    updated_at = <Date 2020-11-23.15:19:16.522>
    user = 'https://github.com/hauntsaninja'

    bugs.python.org fields:

    activity = <Date 2020-11-23.15:19:16.522>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = []
    creation = <Date 2020-10-20.21:48:49.801>
    creator = 'hauntsaninja'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 42102
    keywords = ['patch']
    message_count = 15.0
    messages = ['379169', '379179', '379182', '379196', '379197', '381072', '381080', '381081', '381141', '381151', '381155', '381158', '381641', '381657', '381671']
    nosy_count = 6.0
    nosy_names = ['gvanrossum', 'serhiy.storchaka', 'levkivskyi', 'BTaskaya', 'hauntsaninja', 'kj']
    pr_nums = ['22848']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'enhancement'
    url = 'https://bugs.python.org/issue42102'
    versions = ['Python 3.10']

    @hauntsaninja
    Copy link
    Contributor Author

    In the post PEP-585 world, it seems like a bit of a stumbling block for builtins.callable to not able to be parametrised (in a runtime context). Post PEP-604, I'd expect typing.Callable to be the most used typing import after typing.Any, so much of PEP-585's rationale should apply to this case too.

    Concretely, one way to implement this would be to turn callable into a type whose new returns a bool and which implements class_getitem. We could throw in an instancecheck so that isinstance(x, callable) == callable(x).

    PS: Despite being code that crashes instantly, I was still able to find some instances of isinstance(x, callable) on popular Github projects: https://grep.app/search?q=isinstance%5C%28.%2A%2C%20callable%5C%29&regexp=true&case=true

    @hauntsaninja hauntsaninja added 3.10 only security fixes type-feature A feature request or enhancement labels Oct 20, 2020
    @Fidget-Spinner
    Copy link
    Member

    +1 to this. I'm more surprised that callable wasn't already able to do that (also surprised at the isinstance(x, callable) code crashing).

    I think the implementation would be slightly different than what you proposed. I'd like to give it a shot if you aren't already doing this, and if there's more support for this too.

    @gvanrossum
    Copy link
    Member

    I'm not against this, though I'm not sure it really buys us much (Any still has to be imported from typing). Maybe you can submit a PR? Running the tests might be informative.

    @serhiy-storchaka
    Copy link
    Member

    Callable is very special generic. It differs in many ways from List or Awaitable. You cannot just use GenericAlias.

    Also, in contrary to list() or dict(), callable() is not a constructor, it is a predicate. callable[int], str does not make sense.

    @hauntsaninja
    Copy link
    Contributor Author

    Sure, I put together a draft PR here: #22848 Tests pass cleanly, though timeit indicates callable is slower.

    I haven't added instancecheck, which we should consider doing, since isinstance(x, callable) would now return False, instead of raising TypeError.

    @serhiy
    I hack around that using callable.__new__, so callable[[int], str]() raises a TypeError in my draft PR. Note that callable[[int], str](f) would work the same as callable(f) does today (which from a typechecking perspective is similar to the fact that list[int](("a", "b", "c")) works).
    I agree that callable being a predicate means that there probably isn't an aesthetically pleasing way of doing this.

    @kj
    I'm a little out of my comfort zone, so please let me know if you see a better way of doing things :-)

    @gvanrossum
    Copy link
    Member

    I'd like to pursue this for real; other issues for callable have popped up, https://bugs.python.org/issue42195 and https://bugs.python.org/issue40494 (https://bugs.python.org/issue40398 is also related but already fixed).

    From 42195 I learn that __args__ ought to be hashable. I would prefer it to still be structured, e.g. callable[[int, str], float].__args__ should be ((int, str), float).

    This means we have to change typing.Callable and collections.abc.Callable as well (the latter may share code with builtins.callable, but typing.Callable should probably stay separate, but returning the same structure).

    @serhiy-storchaka
    Copy link
    Member

    Sorry, but making builtins.callable generic looks wrong to me. It is a predicate, not a constructor. If it would be called "iscallable" instead of "callable" nobody would propose to make it generic, right? It's just a coincidence that the name of this predicate equals to the name of typing.Callable and collections.abc.Callable.

    builtins.callable was removed in Python 3.0 in favor of instance check for collections.Callable. Maybe removing it again (or renaming to iscallable) would solve confusion?

    @isidentical
    Copy link
    Member

    Sorry, but making builtins.callable generic looks wrong to me. It is a predicate, not a constructor. If it would be called "iscallable" instead of "callable" nobody would propose to make it generic, right? It's just a coincidence that the name of this predicate equals to the name of typing.Callable and collections.abc.Callable.

    I concur with Serhiy on this.

    @gvanrossum
    Copy link
    Member

    Well, it's certainly no bug fix, but just as PEP-585 lets us write list[int] instead of typing.List[int], it could be considered useful to be able to write callable[[int, int], str] instead of typing.Callable[[int, int], str].

    It's easy enough to make it work so that callable(x) returns a bool but callable[X, Y] returns a built-in subclass of types.GenericAlias (the built-in type).

    That said, I don't have data about how popular Callable is compared to other types (Sequence/Iterable etc. which will remain in collections.abc). Maybe someone can do some grepping of popular projects?

    @serhiy-storchaka
    Copy link
    Member

    From implementation perspective it is not easy at all. You will need to create a special class with methods __call__ and __getitem__ (and several other methods and attributes, like __repr__, __reduce__, __name__, __doc__, __module__, __text_signature__, etc) and make builtins.callable an instance instead of just built-in function. It can also affect performance of callable().

    @hauntsaninja
    Copy link
    Contributor Author

    hauntsaninja commented Nov 16, 2020

    My implementation in PR 22848 turns callable into a type and uses __new__. It isn't too bad. It does appear to affect performance of callable though.

    Here's some data on how often typing imports are used. This is the number of files in which from typing import X is present in mypy_primer's corpus (searched using the AST). My read of this is that Callable is meaningfully more used than anything else in collections.abc.

    'Optional': 2563
    'Any': 2402
    'List': 2383
    'Dict': 2150
    'Tuple': 1592
    'Union': 1269
    'Callable': 1058
    'Iterable': 609
    'Set': 580
    'cast': 505
    'Sequence': 465
    'Type': 438
    'Iterator': 385
    'TYPE_CHECKING': 357
    'TypeVar': 302
    'Mapping': 301
    'Generator': 194
    'NamedTuple': 138
    'Text': 127
    'IO': 120
    'Awaitable': 116
    ...
    

    @serhiy-storchaka
    Copy link
    Member

    We specially introduced __mro_entries__ to make types in the typing module not classes. Turning builtins.callable into a class is a step back.

    @gvanrossum
    Copy link
    Member

    Hm. Shantanu's list shows that the next thing we should make usable without importing typing is Any. (I haven't any idea how to do that other than just making it a builtin.) But after that we should definitely tackle Callable, and the obvious way to do it is to make callable indexable. But does that mean it has to be a type? I don't think so -- it just has to be an object whose class defines both __call__ and __getitem__. Pseudo code:

    class callable:
        def __call__(self, thing):
            return hasattr(thing, "__call__")
        def __getitem__(self, index):
            # returns a types.GenericAlias instance
            # (or a subclass thereof)

    I honestly don't think that we should support isinstance(x, callable) even if some people think that that should work.

    In any case, we should first answer the questions that are still open for bpo-42195 -- what should __args__ for [cC]allable[[int, int], str] be? (int, int, str) or ((int, int), str) or ([int, int], str) or (Tuple[int, int], str) are all still on the table. Please refer to that issue.

    @serhiy-storchaka
    Copy link
    Member

    Could not object replace Any?

    @gvanrossum
    Copy link
    Member

    No, they both have a different meaning. Object has (almost) no attributes. Any has all attributes.

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    topic-typing type-feature A feature request or enhancement
    Projects
    None yet
    Development

    No branches or pull requests

    6 participants