Skip to content
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

Add support for PEP 612 (Parameter Specification Variables) to typing.py #85731

Closed
gvanrossum opened this issue Aug 15, 2020 · 22 comments
Closed

Add support for PEP 612 (Parameter Specification Variables) to typing.py #85731

gvanrossum opened this issue Aug 15, 2020 · 22 comments
Labels
3.10 stdlib type-bug

Comments

@gvanrossum
Copy link
Member

@gvanrossum gvanrossum commented Aug 15, 2020

BPO 41559
Nosy @gvanrossum, @gvanrossum, @Fidget-Spinner
PRs
  • #23702
  • #24000
  • #24056
  • #25449
  • 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 = <Date 2021-04-28.15:47:23.848>
    created_at = <Date 2020-08-15.18:36:04.607>
    labels = ['type-bug', 'library', '3.10']
    title = 'Add support for PEP 612 (Parameter Specification Variables) to typing.py'
    updated_at = <Date 2021-04-28.16:14:24.696>
    user = 'https://github.com/gvanrossum'

    bugs.python.org fields:

    activity = <Date 2021-04-28.16:14:24.696>
    actor = 'gvanrossum'
    assignee = 'none'
    closed = True
    closed_date = <Date 2021-04-28.15:47:23.848>
    closer = 'kj'
    components = ['Library (Lib)']
    creation = <Date 2020-08-15.18:36:04.607>
    creator = 'gvanrossum'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 41559
    keywords = ['patch']
    message_count = 22.0
    messages = ['375487', '382764', '382778', '382805', '382833', '382837', '382843', '382856', '382858', '382938', '383674', '383675', '383676', '384212', '384213', '384217', '386903', '391269', '391270', '392223', '392228', '392231']
    nosy_count = 3.0
    nosy_names = ['gvanrossum', 'Guido.van.Rossum', 'kj']
    pr_nums = ['23702', '24000', '24056', '25449']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue41559'
    versions = ['Python 3.10']

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Aug 15, 2020

    We need stub versions of ParamSpec and Concatenate added to typing.py, plus tests that ensure these actually work in all situations required by the PEP. (It's not so important to ensure that they raise exceptions at runtime in cases where the PEP says they needn't work -- static type checkers will flag those better.)

    @gvanrossum gvanrossum added 3.10 stdlib type-bug labels Aug 15, 2020
    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 8, 2020

    Thanks for working on this -- let me know when you have a question for me. Once this is ready we should also add it to the typing_extensions module so people can use it on older Python versions. (https://github.com/python/typing/tree/master/typing_extensions)

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Dec 8, 2020

    I have a one question:
    Should ParamSpec be treated as a type of TypeVar? This mostly pertains to
    adding it to __parameters__ of generic aliases. Type substitution/chaining,
    seems inappropiate for it right now.

    Thanks for your help!

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 9, 2020

    I wrote

    class C(Generic[T, P]): ...

    and was surprised that C.__parameters__ was (T,) instead of (T, P).

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Dec 10, 2020

    and was surprised that C.__parameters__ was (T,) instead of (T, P).
    Alright, I just fixed that :).

    I'm now encountering many problems with how typing works which prevent what PEP-612 declare as valid from not throwing an error (these are all examples copied from the PEP)::

    class X(Generic[T, P]): ...
    1. X[int, [int, bool]] raises TypeError because second arg isn't a type (this is easy to fix).

    2. X[int, ...] raises TypeError because for the same reason. One way to fix this is to automatically convert Ellipsis to EllipsisType just like how we already convert None to NoneType. Only problem now is that Callable[[...], int] doesn't raise TypeError. Should we just defer the problem of validating Callable to static type checkers instead?

    class Z(Generic[P]): ...
    1. Z[[int, str, bool]] raises TypeError, same as 1.

    2. Z[int, str, bool] raises TypeError, but for too many parameters (not enough TypeVars). This one troubles me the most, current TypeVar substitution checks for len(args) == len(parameters).

    I have a feeling your wish of greatly loosening type checks will come true ;). Should we proceed with that? That'd fix 1, 2, 3. The only showstopper now is 4. A quick idea is to just disable the check for len(args) == len(parameters) if there's only a single ParamSpec in __parameters__.

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 10, 2020

    class X(Generic[T, P]): ...

    1. X[int, [int, bool]] raises TypeError because second arg isn't a type (this is easy to fix).

    How would you fix it? By just allowing it? But then X.__args__ would be unhashable (see the other issue we're working on together).

    1. X[int, ...] raises TypeError because for the same reason. One way to fix this is to automatically convert Ellipsis to EllipsisType just like how we already convert None to NoneType. Only problem now is that Callable[[...], int] doesn't raise TypeError. Should we just defer the problem of validating Callable to static type checkers instead?

    I don't like using EllipsisType here, because this notation doesn't mean that we accept an ellipsis here -- it means (if I understand the PEP correctly) that we don't care about the paramspec.

    class Z(Generic[P]): ...

    1. Z[[int, str, bool]] raises TypeError, same as 1.

    Same comment.

    1. Z[int, str, bool] raises TypeError, but for too many parameters (not enough TypeVars). This one troubles me the most, current TypeVar substitution checks for len(args) == len(parameters).

    The code should check that __parameters__ is (P,) for some ParamSpec P, and then transform this internally as if it was written Z[[int, str, bool]] and then typecheck that.

    **BUT...**

    How about an alternative implementation where as soon as we see that there's a ParamSpec in __parameters__ we just don't type-check at all, and accept anything? Leave it to the static type checker. Our goal here should be to support the syntax that PEP-612 describes so static type checkers can implement it, not to implement all the requirements described by the PEP at runtime.

    I wonder what Pyre has in its stub files for ParamSpec -- maybe we can borrow that?

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 10, 2020

    For reference, here's what Pyre has (though it's an older version):

    https://github.com/facebook/pyre-check/tree/master/pyre_extensions

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Dec 11, 2020

    The pyre version in their __init__.py looks like they took your advice for
    letting the static checker do the work wholeheartedly.

    I'm not in favour of type checking either. Just that the pre-existing code
    does it for me.

    Not type checking when seeing ~P in __parameters__ would work, just that
    __args__ will be unhashable (like you mentioned) so things will be slower
    due to no type cache. Maybe we can cast the [int, str] to (int, str), that
    should work with cache for most cases. And unlike the Callable issue -
    since we don't need to ensure runtime correctness - we can ignore any weird
    effects to __args__ and __parameters__ in ParamSpec, like TypeVars not
    collecting etc.

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 11, 2020

    I started doing this in the original code (long ago when PEP-484 was brand
    new) but have since realized that this makes the typing module both slow
    and hard to maintain. We should not follow this example. I do think we
    should try to keep __args__ hashable, casting [int, str] to (int, str) sounds right.

    @rhettinger rhettinger changed the title Add support for PEP 612 to typing.py Add support for PEP 612 to typing.py Dec 11, 2020
    @rhettinger rhettinger changed the title Add support for PEP 612 to typing.py Add support for PEP 612 to typing.py Dec 11, 2020
    @rhettinger rhettinger changed the title Add support for PEP 612 to typing.py Add support for PEP 612 (Parameter Specification Variables) to typing.py Dec 11, 2020
    @rhettinger rhettinger changed the title Add support for PEP 612 to typing.py Add support for PEP 612 (Parameter Specification Variables) to typing.py Dec 11, 2020
    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 13, 2020

    This is now unblocked now that #67249 has landed.

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 24, 2020

    New changeset 73607be by kj in branch 'master':
    bpo-41559: Implement PEP-612 - Add ParamSpec and Concatenate to typing (bpo-23702)
    73607be

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Dec 24, 2020

    Huge thanks!

    I think the next step is to port the essence to typing_extensions, which has to work for anything from 3.5 up (or maybe 3.6), *including* 3.10 and above.

    https://github.com/python/typing/tree/master/typing_extensions

    Honestly maybe we could just make ParamSpec an alias for TypeVar there, and make Concatenate an alias for Tuple? Because "work" just means that the syntax needs to be valid, the type checkers are responsible for using it. (We're still working on getting this to work for mypy.)

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Dec 24, 2020

    Thanks for the extremely helpful reviews and help in this Guido!

    Sure, I'll probably start work on that next week, slightly busy with life right now. After that I'll work on docs.

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Jan 2, 2021

    New changeset 11276cd by Ken Jin in branch 'master':
    bpo-41559: Documentation for PEP-612 (GH-24000)
    11276cd

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Jan 2, 2021

    Looks like we can close this now, right? You can open a separate issue in the python/typing repo to update typing_extensions.

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Jan 2, 2021

    I have just one more PR - one that converts genericalias nested lists to
    tuples by default. It'll simplify the code a little bit and prepare 3.9.x
    for PEP-612.

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Feb 13, 2021

    Hi Guido, after a month of observing how people stumbled over the related collections.abc.Callable bugs, and experience from implementing this PEP, I learnt a few things which I think may interest you:

    Previously it was *recommended* that everything in typing be hashable. I would now say it is *required*. Union uses sets to de-duplicate arguments, and Optional uses Union internally. Both blow up if things aren't hashable. A surprising number of people caught the collections.abc.Callable bug because Optional[Callable[.....]] failed.

    Going forward, future PEPs to typing.py probably need to ensure their implementations are hashable, or risk not working with some of the types in the module itself. Alternatively, they can always change the implementations of Union and Optional, though I don't know if I recommend that ;).

    Thanks for your time.

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Apr 17, 2021

    Guido, I hope I didn't choose a bad time to send this PR over (I suspect you may already be flooded by emails about PEP-563).

    The jist of the PR is that it's possible to implement PEP-612 in pure-Python. Specifically, PEP-612 says:

    As before, parameters_expressions by themselves are not acceptable in places where a type is expected

    https://www.python.org/dev/peps/pep-0612/#valid-use-locations

    Currently, the implementation treats ParamSpec specially in all builtin GenericAlias objects just for the sake of collections.abc.Callable. There isn't a need for that - it just needs collections.abc.Callable to exhibit that behaviour.

    By implementing this in pure Python, we can:

    • Conform more strictly to the PEP.
    • Reduce complexity of the builtin GenericAlias and also speed it up because less checks are required.
    • Not tie more typing.py stuff to builtin GenericAlias, which is a plus in my book.

    What do you think?

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Apr 17, 2021

    Yeah, like this idea. Let’s get this in before beta 1.

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Apr 28, 2021

    New changeset 859577c by Ken Jin in branch 'master':
    bpo-41559: Change PEP-612 implementation to pure Python (bpo-25449)
    859577c

    @Fidget-Spinner
    Copy link
    Member

    @Fidget-Spinner Fidget-Spinner commented Apr 28, 2021

    The last of the patches have landed. Guido, thank you so much for helping me through this 5 month long process. Please enjoy your vacation!

    PS: I need to send in a bugfix for typing.py later to ignore ParamSpec in the __parameters__ of invalid locations like typing.List (this just ensures consistency with the builtin list). That can be done as a bugfix patch after the 3.10 beta freeze (as it isn't a new feature at all). I'll open a new issue after beta 1 for that when you're back. Thanks!

    @gvanrossum
    Copy link
    Member Author

    @gvanrossum gvanrossum commented Apr 28, 2021

    Thanks for your major contribution, Ken! Agreed, that bugfix can come later.

    @ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.10 stdlib type-bug
    Projects
    None yet
    Development

    No branches or pull requests

    2 participants