| FazBrowse GitHub Viewer | Trending | | Home |
| Tools: [Download Repo ZIP] [Original HTTPS Page] |
Co-authored-by: Donghee Na <donghee.na@python.org>
|
This PR only changes private names, so it should not impact stdlib users. |
Sorry, something went wrong.
|
If this change is merged, I will propose a follow-up PR which modify public names. |
Sorry, something went wrong.
There was a problem hiding this comment.
One optional suggestion in typing.py
Sorry, something went wrong.
There was a problem hiding this comment.
Ok for ssl and a suggestion for opcode
Sorry, something went wrong.
| _cache_format = { | ||
| "LOAD_GLOBAL": { | ||
| _cache_format = frozendict( | ||
| LOAD_GLOBAL={ |
There was a problem hiding this comment.
The values of LOAD_GLOBAL can also be a frozendict (that is, we can have nested frozendicts).
Sorry, something went wrong.
There was a problem hiding this comment.
ditto: #144910 (review)
Sorry, something went wrong.
|
The SC does not allow mechanically replacing all uses of dict. Please wait for reviews from the code owners of each module. |
Sorry, something went wrong.
|
What's the benefit of changing this in e.g. typing.py? It might break some users who modify this dict. That's not a supported use case, but we shouldn't go out of our way to break users unless there's some benefit. |
Sorry, something went wrong.
The purpose of this change is to prevent dictionary modifications, especially if the modification is done by mistake. |
Sorry, something went wrong.
|
@JelleZijlstra: Do you prefer that I revert the typing._PROTO_ALLOWLIST change? |
Sorry, something went wrong.
|
While the variables in stringprep are public, note that the file is autogenerated: # This file is generated by mkstringprep.py. DO NOT EDIT.And the docs clearly state:
So, I really think we should consider any direct use or mutation of those variables as really a bad idea and I don't think we should even consider code that messes with that as being legitimate. |
Sorry, something went wrong.
I revert the stringprep change to restrict this change to private variables. Changing private variables type should be less controversial than changing public variables type. |
Sorry, something went wrong.
|
I reverted the typing.py changes. I plan to merge this PR at the end of the week, unless there is a strong reason to not make these changes. |
Sorry, something went wrong.
|
If someone really want/need to modify one of the frozendict, it remains possible. Example: platform._ver_stages |= frozendict(final=60) $ ./python
>>> import platform
>>> platform._ver_stages
frozendict({'dev': 10, 'alpha': 20, 'a': 20, 'beta': 30, ..., 'p': 200})
>>> platform._ver_stages['final']=60
...
TypeError: 'frozendict' object does not support item assignment
>>> platform._ver_stages |= frozendict(final=60)
>>> platform._ver_stages
frozendict({'dev': 10, 'alpha': 20, 'a': 20, 'beta': 30, ..., 'p': 200, 'final': 60})The purpose of this change is more to catch dictionaries modified by accident. |
Sorry, something went wrong.
|
I merged my PR, thanks for reviews. If someone comes up with an concrete use case to modify one of these private attributes, we can revert the affected attribute. |
Sorry, something went wrong.
Co-authored-by: Donghee Na <donghee.na@python.org>
| Back | FazBrowse Home | New Git URL |
Uh oh!
There was an error while loading. Please reload this page.
Sorry, something went wrong.
Uh oh!
There was an error while loading. Please reload this page.