Skip to content

_typevar_types and _paramspec_tvars are missing from _GenericAlias.copy_with #90739

Closed
Closed
_typevar_types and _paramspec_tvars are missing from _GenericAlias.copy_with#90739
Labels
3.10 (EOL)end of life3.11only security fixesstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

Description

@posita
posita
mannequin
opened on Jan 30, 2022
BPO 46581
Nosy @gvanrossum, @posita, @serhiy-storchaka, @JelleZijlstra, @posita, @sobolevn, @Fidget-Spinner
PRs
  • bpo-46581: Propagate private vars via _GenericAlias.copy_with #31061
  • [3.10] bpo-46581: Propagate private vars via _GenericAlias.copy_with (GH-31061) #31821
    		•

    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-11.14:58:44.809>
created_at = <Date 2022-01-30.13:20:18.515>
labels = ['type-bug', 'library', '3.10', '3.11']
title = '_typevar_types and _paramspec_tvars are missing from _GenericAlias.copy_with'
updated_at = <Date 2022-03-11.14:58:44.808>
user = 'https://github.com/posita'

    bugs.python.org fields:

    activity = <Date 2022-03-11.14:58:44.808>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2022-03-11.14:58:44.809>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2022-01-30.13:20:18.515>
creator = 'posita'
dependencies = []
files = []
hgrepos = []
issue_num = 46581
keywords = ['patch']
message_count = 8.0
messages = ['412144', '412145', '412149', '412266', '412274', '412283', '414850', '414910']
nosy_count = 7.0
nosy_names = ['gvanrossum', 'mbogosian', 'serhiy.storchaka', 'JelleZijlstra', 'posita', 'sobolevn', 'kj']
pr_nums = ['31061', '31821']
priority = 'normal'
resolution = 'fixed'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue46581'
versions = ['Python 3.10', 'Python 3.11']

    Activity

    1. posita commented on Jan 30, 2022

      @posita
      positamannequin
      on Jan 30, 2022
      MannequinAuthor

      c55ff1b was submitted to fix bpo-44098 and added the _typevar_types and _paramspec_tvars properties to _GenericAlias. However, those properties continue to be omitted from _GenericAlias.copy_with1.

      Further, typing.py is fairly intricate, which makes it hard to understand if that is the only place those properties are absent. A more careful review/audit may be in order.

    2. posita
      added
      3.10 (EOL)end of life
      3.11only security fixes
      stdlibStandard Library Python modules in the Lib/ directory
      on Jan 30, 2022

    3. posita commented on Jan 30, 2022

      @posita
      positamannequin
      on Jan 30, 2022
      MannequinAuthor

      Filed by request: #26091 (comment)

    4. AlexWaygood
      added
      type-bugAn unexpected behavior, bug, or error
      on Jan 30, 2022

    5. Fidget-Spinner commented on Jan 30, 2022

      @Fidget-Spinner
      Fidget-Spinner
      on Jan 30, 2022
      Member

      Wow! Thanks, that's an interesting find. My hunch is that we should be passing in _typevar_types and _paramspec_tvars in the copy_with of _GenericAlias and _ConcatenateGenericAlias. Inconsistent copy_with could trigger subtle bugs in ForwardRefs and get_type_hints. I can reproduce a semi-bug right now:

      >>> P = ParamSpec('P')
>>> Callable[P, int].__parameters__
(~P,)
>>> Callable[P, int].copy_with((P,int))
typing.Callable[~P, int]
>>> Callable[P, int].copy_with((P,int)).__parameters__
()
^ This shouldn't be empty!!

      Would you like to submit a patch for this?

    6. posita commented on Feb 1, 2022

      @posita
      positamannequin
      on Feb 1, 2022
      MannequinAuthor

      I am happy to attempt a patch, but I don't understand what's going on with _ConcatenateGenericAlias. Or rather, I don't fully understand the various copy_with semantics. This code is *very* hard to follow.

      Patching _GenericAlias.copy_with seems relatively straightforward:

           def copy_with(self, params):
-        return self.__class__(self.__origin__, params, name=self._name, inst=self._inst)
+        return self.__class__(self.__origin__, params, name=self._name, inst=self._inst,
+                              _typevar_types=self._typevar_types,
+                              _paramspec_tvars=self._paramspec_tvars)

      _ConcatenateGenericAlias.copy_with, on the other hand, appears to return a tuple rather than a type until it falls back to the parent implementation. What does one do with that?

      Also, what about _AnnotatedAlias, _SpecialGenericAlias, _UnionGenericAlias, or _strip_annotations? Do any of those need to be modified?

      I can't find any tests that deal with copy_with directly, so I'm assuming its use is stressed via higher level code paths, but it's not clear what use cases that method is supposed to serve. The good news is that its use is confined to typing.py. The bad news is that file gives little insight to those who aren't already experts in that territory.

      In short, I will do my best, but I suspect I will need patience and guidance in arriving something acceptable.

    7. Fidget-Spinner commented on Feb 1, 2022

      @Fidget-Spinner
      Fidget-Spinner
      on Feb 1, 2022
      Member

      In short, I will do my best, but I suspect I will need patience and guidance in arriving something acceptable.

      Yeah no worries. typing.py is really complex for many historical reasons. I still don't grasp all of it and probably never will.

      what about _AnnotatedAlias, _SpecialGenericAlias, _UnionGenericAlias, or _strip_annotations?

      They shouldn't need modification. And there's an elegant reason why. Currently ParamSpec is only valid in Generic, Callable and Concatenate. We don't care about any other types. As long as ParamSpec appears in their __parameters__ and the copy_with of those 3 have the correct settings.

      Well what about when they're nested? The other types blindly copy over whatever they see in __parameters__ of any nested types. So even if you don't pass those settings to their copy_with, it doesn't matter (well of course, until someone else uses this setting and expands its scope, then proceeds to trip over it :(. this seems like a potential trap affecting PEP-646's runtime.)

      >>> X = Concatenate[int, ParamSpec('P')]
>>> Container[X]
typing.Container[typing.Concatenate[int, ~P]]
>>> Container[X].copy_with(X).__parameters__
(~P,)
^ Works even though this uses _SpecialGenericAlias, which doesn't properly pass in the settings!

      I can't find any tests that deal with copy_with directly, so I'm assuming its use is stressed via higher level code paths, but it's not clear what use cases that method is supposed to serve.

      Yeah. If you search for "copy_with" in typing.py. You'll find that it's called in __getitem__. Now __getitem__ is used when we subscript an already subscripted type. What I mean is:

      T = TypeVar('T')
X = List[T] (__class_getitem__ is called, returning a new genericalias object)
X[int] (__getitem__ is called, returning a new genericalias object)

      We need copy_with because we want to create a full "copy" of all the args into the new GenericAlias object and not face weird problems with mutability. That specific use case is tested super often.

      _ConcatenateGenericAlias.copy_with, on the other hand, appears to return a tuple rather than a type until it falls back to the parent implementation. What does one do with that?

      Frankly, I was quite lost too until I saw that it was part of bpo-44791, which deals with special special (undefined) cases in PEP-612 and Concatenate. Basically for a very weird Concatenate, the core dev there has chosen to return a tuple of types instead of another type. Let's just ignore those strange tuple paths, and only care about the path using the parent implementation.

    8. posita commented on Feb 1, 2022

      @posita
      positamannequin
      on Feb 1, 2022
      MannequinAuthor

      Thanks, @kj! Fantastic education and insight! I'm sad that I needed you as an interpreter but very grateful you were around to provide the interpretation. Working on a patch now….

    9. serhiy-storchaka commented on Mar 10, 2022

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 10, 2022
      Member

      New changeset 32bf359 by Matt Bogosian in branch 'main':
      bpo-46581: Propagate private vars via _GenericAlias.copy_with (GH-31061)
      32bf359

    10. serhiy-storchaka commented on Mar 11, 2022

      @serhiy-storchaka
      serhiy-storchaka
      on Mar 11, 2022
      Member

      New changeset 3bc8019 by Matt Bogosian in branch '3.10':
      [3.10] bpo-46581: Propagate private vars via _GenericAlias.copy_with (GH-31061) (GH-31821)
      3bc8019

    11. serhiy-storchaka
      closed this as completedon Mar 11, 2022
    12. ezio-melotti
      transferred this issue fromon Apr 10, 2022
    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

    Metadata

    Metadata

    Assignees

    No one assigned

      Labels

      3.10 (EOL)end of life3.11only security fixesstdlibStandard Library Python modules in the Lib/ directorytype-bugAn unexpected behavior, bug, or error

      Projects

      No projects

        Milestone

        No milestone

        Relationships

        None yet

        Development

        No branches or pull requests

        Issue actions