Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork34.2k
gh-145119: Allow frozendict to be assigned to instance __dict__#145123
gh-145119: Allow frozendict to be assigned to instance __dict__#145123mohsinm-dev wants to merge 2 commits intopython:mainfrom
Conversation
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.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit:6b702cf72b
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| _PyObject_SetDict(PyObject *obj, PyObject *value) | ||
| { | ||
| if (value != NULL && !PyDict_Check(value)) { | ||
| if (value != NULL && !PyAnyDict_Check(value)) { |
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.
Avoid frozendict dict on objects mutating their own dict
Allowing_PyObject_SetDict() to acceptPyAnyDict_Check here makesfrozendict assignable to everyPyObject_GenericSetDict user, but some of those types still perform internal writes withPyDict_SetItem/PyDict_DelItem (for exampledescriptor_get_wrapped_attribute anddescriptor_set_wrapped_attribute inObjects/funcobject.c for classmethod/staticmethod annotation caching).PyDict_SetItem still rejects frozendict withPyErr_BadInternalCall, so afterobj.__dict__ = frozendict(...), operations like reading or updating__annotations__ can now raiseSystemError instead of behaving normally or reporting an intentional immutability error.
Useful? React with 👍 / 👎.
6b702cf to8d6ec8dCompareskirpichev commentedMar 4, 2026
In order to keep the commit history intact,please avoid squashing or amending history and then force-pushing to the PR. Reviewers often want to look at individual commits. When the PR is merged, everything will be squashed into a single commit. |
| Py_SETREF(dict, temp); | ||
| } | ||
| else if (PyFrozenDict_Check(dict)) { | ||
| /* Convert frozendict to a mutable dict for merging. */ |
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.
Please wait for#145517. If PyDict_Copy() is modified to always return a dict, you can reuse the PyDict_Check() branch for frozendict.
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.
For now, you can use_PyDict_CopyAsDict() fromInclude/internal/pycore_dict.h.
Objects/typeobject.c Outdated
| "__dict__ must be set to a dictionary, " | ||
| "not a '%.200s'", Py_TYPE(value)->tp_name); |
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.
| "__dict__ must be set to adictionary, " | |
| "not a'%.200s'",Py_TYPE(value)->tp_name); | |
| "__dict__ must be set to adict or frozendict, " | |
| "not a%T",value); |
vstinner commentedMar 5, 2026
I extracted these changes from your PR and created a new PR based on it: PRgh-145564. |
Uh oh!
There was an error while loading.Please reload this page.
Allow
frozendict(and its subclasses) to be assigned to an instance's__dict__, enabling immutable instances.Changes:
_PyObject_SetDict: acceptfrozendictviaPyAnyDict_Check_PyDict_SetItem_LockHeld: raiseTypeErroron set/delete instead ofSystemErrorobject___dir___impl: convertfrozendictto a mutable dict for merging