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

typing.Annotated cannot wrap typing.ParamSpec args/kwargs #90801

Closed
GBeauregard mannequin opened this issue Feb 4, 2022 · 9 comments
Closed

typing.Annotated cannot wrap typing.ParamSpec args/kwargs #90801

GBeauregard mannequin opened this issue Feb 4, 2022 · 9 comments
Labels
3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error

Comments

@GBeauregard
Copy link
Mannequin

GBeauregard mannequin commented Feb 4, 2022

BPO 46643
Nosy @gvanrossum, @serhiy-storchaka, @JelleZijlstra, @miss-islington, @sobolevn, @Fidget-Spinner, @AlexWaygood, @GBeauregard
PRs
  • bpo-46643: Fix stringized P.args/P.kwargs with get_type_hints #31238
  • [3.10] bpo-46643: Fix stringized P.args/P.kwargs with get_type_hints (GH-31238) #31646
  • bpo-46643: fix NEWS entry #31651
  • 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 2022-03-03.01:22:04.048>
    created_at = <Date 2022-02-04.22:19:25.985>
    labels = ['type-bug', 'library', '3.9', '3.10', '3.11']
    title = 'typing.Annotated cannot wrap typing.ParamSpec args/kwargs'
    updated_at = <Date 2022-03-03.05:27:28.381>
    user = 'https://github.com/GBeauregard'

    bugs.python.org fields:

    activity = <Date 2022-03-03.05:27:28.381>
    actor = 'JelleZijlstra'
    assignee = 'none'
    closed = True
    closed_date = <Date 2022-03-03.01:22:04.048>
    closer = 'JelleZijlstra'
    components = ['Library (Lib)']
    creation = <Date 2022-02-04.22:19:25.985>
    creator = 'GBeauregard'
    dependencies = []
    files = []
    hgrepos = []
    issue_num = 46643
    keywords = ['patch']
    message_count = 9.0
    messages = ['412546', '412549', '412817', '412818', '412869', '412973', '414394', '414399', '414404']
    nosy_count = 8.0
    nosy_names = ['gvanrossum', 'serhiy.storchaka', 'JelleZijlstra', 'miss-islington', 'sobolevn', 'kj', 'AlexWaygood', 'GBeauregard']
    pr_nums = ['31238', '31646', '31651']
    priority = 'normal'
    resolution = 'fixed'
    stage = 'resolved'
    status = 'closed'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue46643'
    versions = ['Python 3.9', 'Python 3.10', 'Python 3.11']

    @GBeauregard
    Copy link
    Mannequin Author

    GBeauregard mannequin commented Feb 4, 2022

    Consider the following.

    import logging
    from typing import Annotated, Callable, ParamSpec, TypeVar
    
    T = TypeVar("T")
    P = ParamSpec("P")
    
    def add_logging(f: Callable[P, T]) -> Callable[P, T]:
        """A type-safe decorator to add logging to a function."""
        def inner(*args: Annotated[P.args, "meta"], **kwargs: P.kwargs) -> T:
            logging.info(f"{f.__name__} was called")
            return f(*args, **kwargs)
        return inner
    
    @add_logging
    def add_two(x: float, y: float) -> float:
        """Add two numbers together."""
        return x + y
    

    This raises an error at runtime because P.args/P.kwargs cannot pass the typing._type_check called by Annotated because they are not callable(). This prevents being able to use Annotated on these type annotations.

    This can be fixed by adding __call__ methods that raise to typing.ParamSpecArgs and typing.ParamSpecKwargs to match other typeforms.

    I can write this patch given agreement

    @GBeauregard GBeauregard mannequin added 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels Feb 4, 2022
    @GBeauregard
    Copy link
    Mannequin Author

    GBeauregard mannequin commented Feb 4, 2022

    We can also fix this with my proposal in bpo-46644. I'm okay with either fix (that or implementing __call__), or both.

    @serhiy-storchaka
    Copy link
    Member

    Should it? Annotated wraps types, and P.args/P.kwargs are not types.

    If make Annotated accepting P.args/P.kwargs (and possible other non-types, e.g. P), it is a new feature which should be publicly discussed first and documented. I do not thing it requires a PEP.

    @GBeauregard
    Copy link
    Mannequin Author

    GBeauregard mannequin commented Feb 8, 2022

    My general understanding has been that since Annotated exists primarily to allow interoperability between typing and non-typing uses of annotation space, and that this is quite clear in the PEP, most cases where this is not allowed (especially at the top level of the annotation, as in this case) are either a bug or unintentional. We fixed Annotated wrapping Final and ClassVar a couple weeks ago for arguments sorta along these lines after discussion on typing-sig, where the concern of what constitutes a type to wrap was specifically addressed: https://bugs.python.org/issue46491

    But, I am not opposed to discussing this case too, or discussing the issue more broadly. Is typing-sig the appropriate place for me to make a thread, or do we wait for comments here? Even if we decide this particular issue is okay to fix, I think you bring up a good point about documenting our decision clearly once and for all so I agree we should do this.

    @GBeauregard
    Copy link
    Mannequin Author

    GBeauregard mannequin commented Feb 8, 2022

    I have made a thread on typing-sig to discuss this: https://mail.python.org/archives/list/typing-sig@python.org/thread/WZ4BCFO4VZ7U4CZ4FSDQNFAKPG2KOGCL/

    @GBeauregard
    Copy link
    Mannequin Author

    GBeauregard mannequin commented Feb 10, 2022

    I wrote a PR that fixes the underlying issue here, but I'm leaving it as a draft while the discussion plays out. I think the stuff currently in the patch should be okay regardless of the discussion decision, because the underlying issue is that P.args and P.kwargs didn't pass typing._type_check, which is needed for reasons unrelated to Annotated (such as if they were stringified and had get_type_hints called on them).

    If the result of the discussion is that we need to start supporting limitations on where Annotated is used, I'll add another PR that introduces support for this pattern in whatever locations that are decided it shouldn't be allowed.

    @JelleZijlstra
    Copy link
    Member

    New changeset 75d2d94 by Gregory Beauregard in branch 'main':
    bpo-46643: Fix stringized P.args/P.kwargs with get_type_hints (GH-31238)
    75d2d94

    @miss-islington
    Copy link
    Contributor

    New changeset 257f5be by Miss Islington (bot) in branch '3.10':
    bpo-46643: Fix stringized P.args/P.kwargs with get_type_hints (GH-31238)
    257f5be

    @JelleZijlstra
    Copy link
    Member

    New changeset 59e1ce9 by Jelle Zijlstra in branch 'main':
    bpo-46643: fix NEWS entry (GH-31651)
    59e1ce9

    @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.9 only security fixes 3.10 only security fixes 3.11 only security fixes stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    3 participants