- 
          
- 
                Notifications
    You must be signed in to change notification settings 
- Fork 33.3k
          gh-138859: Account for ParamSpec defaults that are not lists …
          #138868
        
          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
Conversation
| Most changes to Python require a NEWS entry. Add one using the blurb_it web app or the blurb command-line tool. If this change has little impact on Python users, wait for a maintainer to apply the  | 
        
          
                Lib/typing.py
              
                Outdated
          
        
      | elif isinstance(args[i], list): | ||
| args = (*args[:i], tuple(args[i]), *args[i+1:]) | ||
| else: | ||
| args = (*args[:i], args[i], *args[i + 1:]) | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this can be simplified to args = tuple(args)
| @@ -0,0 +1 @@ | |||
| Fix failure during parameterization of generics when a non-type-list `ParamSpec` default is followed by another type variable default, and the `ParamSpec` is not provided as a type argument. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We use rst, not md in NEWS, so please use double backticks.
        
          
                Lib/typing.py
              
                Outdated
          
        
      | # Convert lists to tuples to help other libraries cache the results. | ||
| elif isinstance(args[i], list): | ||
| args = (*args[:i], tuple(args[i]), *args[i+1:]) | ||
| else: | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create a tuple directly on line 1103 above instead?
| @@ -0,0 +1 @@ | |||
| Fix failure during parameterization of generics when a non-type-list ``ParamSpec`` default is followed by another type variable default, and the ``ParamSpec`` is not provided as a type argument. | |||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I don't understand what "non-type-list" is. I propose this wording:
| Fix failure during parameterization of generics when a non-type-list ``ParamSpec`` default is followed by another type variable default, and the ``ParamSpec`` is not provided as a type argument. | |
| Fixes a :exc:`TypeError` when substituting a :class:`ParamSpec` with a default | |
| that is followed by another :class:`TypeVar` with a default. | 
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks, the original description was unclear. Unless there are other objections, I'll reduce it to
Fix generic type parameterization raising a :exc:
TypeErrorwhen omitting a :class:ParamSpecthat has a non-type-list default.
"Non-type-list default" refers to the bug occurring if the ParamSpec default isn't a list of types, such as ... or P (another ParamSpec in scope). The bug would not be encountered if the default was a list of types (like [int, str]).
I think the closest wording for this construct in the documentation is actually "list of types" (see e.g. ParamSpec Defaults), but I don't think "list of types" is necessarily better than "type list".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's say something like "default that is not a list of types". "type list" is definitely less clear to me than "list of types".
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Rewording suggestions done in 9669479
| @bzoracler Something left to do here? Is it ready for review? | 
…when converting type argument list to tuple
9669479    to
    bd6d90f      
    Compare
  
    | @sergey-miryanov I think I've addressed everything. @JelleZijlstra Are there any other concerns? | 
| Thanks @bzoracler for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13, 3.14. | 
pythonGH-138868) (cherry picked from commit 379fd02) Co-authored-by: bzoracler <[email protected]>
pythonGH-138868) (cherry picked from commit 379fd02) Co-authored-by: bzoracler <[email protected]>
| GH-140207 is a backport of this pull request to the 3.14 branch. | 
| GH-140208 is a backport of this pull request to the 3.13 branch. | 
| @bzoracler it looks like you'll have to re-sign the CLA under a different email address for the backports (#140208 and #140207) to go through. As I understand it this can happen if you have different email addresses associated with your GitHub account and your commits. | 
| @JelleZijlstra oh, it looks like rebasing straight from the GitHub UI made the commits under a different email address ( Thanks all! | 
…when converting type argument list to tuple
Allows omitting common
ParamSpecdefaults, like...or anotherParamSpec.ParamSpec(default=...)fails if not provided as a type argument and not in the last position of a type list when passing type arguments #138859ParamSpec(default=...)fails if not provided as a type argument and not in the last position of a type list when passing type arguments #138859