| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Sorry, something went wrong.
|
After some sleep, I suggest renaming TypeVarBoundBlock and TypeParamBlock but keep the three distinctions, at least for compatibility purposes. This change would affect the symtable module, especially symtable.SymbolTable.get_type but there are various issues with the current naming IMO.
def foo[T: int, *Ts = *tuple[int], **P = [int, str]](): passI would say that T, *Ts and **P are "type variables" and that "foo" has 3 "type parameters". In particular, I would like to rename TypeParamBlock into TypeParamsBlock and TypeVarBoundBlock into TypeVarBlock even though it is implicitly assumed that there is a bound/constraints or default for that one. The rationale behind this choice is also motivated by the following test in test_symtable.py: class GenericMine[T: int]:
passThe symbol table's type for GenericMine is "type parameter" (because it's the annotation scope); the symbol table's type for the inner's GenericMine (i.e., the concrete class being created) is "class" (this is fine) and the symbol table's type for "T" is "TypeVar bound", but this would also be its type if I had "[T = int]"... The renaming would affect the current symtable module as follows:
We could also emit a symbol table for type parameters without any bound, constraint or default and the renaming would make better sense. By the way, DEF_TYPE_PARAM is never exported to the Python module and we have something like: >>> import symtable
>>> s = symtable.symtable("class A[T]: pass", "?", "exec")
>>> s.get_children()[0].lookup('T')
<symbol 'T': LOCAL, DEF_LOCAL>where I would have expected T to be have the DEF_TYPE_PARAM flag. I'll open a separate issue for that one. |
Sorry, something went wrong.
|
Thanks! I agree with renaming TypeParamBlock to TypeParamsBlock. For TypeVarBoundBlock, note that it is used for bounds and constraints as well as defaults, and the latter can appear on ParamSpecs and TypeVarTuples as well as TypeVars. Therefore, I think it's better to use the full name "TypeVariableBlock", using "type variable" as a general term for all three flavors. My biggest issue with changing these names is backwards compatibility in the symtable module. We explicitly document a set of strings at https://docs.python.org/3/library/symtable.html#symtable.SymbolTable.get_type. We can add to the set of strings for 3.14 and probably for 3.13, but for 3.12 it should stay the same. |
Sorry, something went wrong.
Yes, that's what I had in mind (actually, I wrote TypeVarBlock but it should have been TypeVariableBlock, so my bad). Since we are anyway using full names, I could also suggest using TypeParametersBlock, unless you think it's too verbose.
3.13 should be fine (I guess) because it's still a pre-release so... but would it make sense to instead return enumeration constants that are publicly accessible instead of strings like that..? at least we wouldn't have the issue in the future if the grammar is extended/changed and we could probably make the attribute deprecated more easily ? |
Sorry, something went wrong.
|
Yes, I think using an enum would make sense and it could be made backwards-compatible by using StrEnum. |
Sorry, something went wrong.
- improve comments - rename 'TypeVarBoundBlock' -> 'TypeVariableBlock' - rename 'TypeParamBlock' -> 'TypeParametersBlock' - rename 'ste_description' -> 'ste_context_info' - only set 'ste_context_info' just before visiting the actual expression
- rename '_symtable.TYPE_TYPE_VAR_BOUND' to '_symtable.TYPE_TYPE_VARIABLE' - rename '_symtable.TYPE_TYPE_PARAM' to '_symtable.TYPE_TYPE_PARAMETERS' - add string enumeration for symbol table type
|
@JelleZijlstra I've updated the proposal. By the way, I've only done that for what I've added but I'll likely re-order the case so that they match the enumeration. I put TypeAliasBlock, TypeParametersBlock and TypeVariableBlock in the enumeration (in that order) to reflect the "top-to-bottom" construction (TypeVariableBlock requires to first enter a TypeParametersBlock). |
Sorry, something went wrong.
There was a problem hiding this comment.
General direction looks fine to me. I won't have time soon to do a detailed review but if @JelleZijlstra is happy with it, that's good enough for me.
Sorry, something went wrong.
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
Thanks @picnixz for the PR, and @JelleZijlstra for merging it 🌮🎉.. I'm working now to backport this PR to: 3.13. |
Sorry, something went wrong.
|
Sorry, @picnixz and @JelleZijlstra, I could not cleanly backport this to 3.13 due to a conflict. cherry_picker 4bf17c381fb7b465f0f26aecb94a6c54cf9be2d3 3.13 |
Sorry, something went wrong.
|
I am doing the backport |
Sorry, something went wrong.
…ype parameters expressions (pythonGH-119976) (cherry picked from commit 4bf17c3) Co-authored-by: Bénédikt Tran <10796600+picnixz@users.noreply.github.com> Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
|
GH-120641 is a backport of this pull request to the 3.13 branch. |
Sorry, something went wrong.
…ameters expressions (python#119976) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…ameters expressions (python#119976) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
…ameters expressions (python#119976) Co-authored-by: Jelle Zijlstra <jelle.zijlstra@gmail.com>
| Back | FazBrowse Home | New Git URL |
cc @JelleZijlstra
I added a bunch of tests but feel free to tell me if there are more cases to check. It took me a bit of time to observe that entering the context was handled inside symtable_visit_type_param_bound_or_default and not upon calling it.
Tell me if I need a What's New entry by the way or if the NEWS entry should be improved.