Conversation
* PyDict_Copy() now also returns a dict if the argument is a frozendict. * Remove _PyDict_CopyAsDict() function. * Fix frozendict.items() ^ frozendict.items(). Add non-regression test.
|
Currently, Existing C extensions calling Always returning |
|
I'm not so sure this is a good idea. If I call something named |
|
The
It would mean that such code (using this PR): PyObject *dict;
dict = PyDict_Copy(orig_dict);
if (dict == NULL) {
goto error;
}should be replaced with: PyObject *dict;
if (PyFrozenDict_Check(orig_dict)) {
dict = PyDict_New();
if (dict == NULL) {
goto error;
}
if (PyDict_Merge(dict, orig_dict, 1) < 0) {
goto error;
}
}
else {
dict = PyDict_Copy(orig_dict);
if (dict == NULL) {
goto error;
}
}As a temporary solution, I added an internal function But I would prefer to have a public C API for such code. |
|
I completed PyDict_Copy() documentation. |
|
Another example in PR gh-145123: else if (PyDict_Check(dict)) {
/* Copy __dict__ to avoid mutating it. */
PyObject *temp = PyDict_Copy(dict);
Py_SETREF(dict, temp);
}
else if (PyFrozenDict_Check(dict)) {
/* Convert frozendict to a mutable dict for merging. */
PyObject *temp = PyDict_New();
if (temp != NULL && PyDict_Update(temp, dict) < 0) {
Py_DECREF(temp);
temp = NULL;
}
Py_SETREF(dict, temp);
} |
And here is something completely different! I wrote #145531 to add |
|
Why is frozen dict needed to be copied? It's immutable. |
|
|
||
| return _PyDict_Copy(o); | ||
| PyObject *res; | ||
| Py_BEGIN_CRITICAL_SECTION(o); |
There was a problem hiding this comment.
Even we want to copy as dict from frozendict, critical section is actually needed for frozendict?
There was a problem hiding this comment.
We should be able to skip the critical section for frozendict, at least for the PyFrozenDict_CheckExact() case. But I would prefer to work on such optimization in a separated PR since this PR (and PR gh-145531) is already quite complex.
Multiple functions accept |
Not disagreeing that it's useful, but it's also a huge footgun. I think it's really counterintuitive for a copy to have a different type than the input. That, and how are users supposed to actually create a copy of I'd be okay with it if we did the following:
With the existence of those two, the expected behavior is much more intuitive. |

📚 Documentation preview 📚: https://cpython-previews--145517.org.readthedocs.build/