bpo-14965: Proxy super().x = y and del super().x (updated)
#29950
Conversation
|
Hello, and thanks for your contribution! I'm a bot set up to make sure that the project can legally accept this contribution by verifying everyone involved has signed the PSF contributor agreement (CLA). Recognized GitHub usernameWe couldn't find a bugs.python.org (b.p.o) account corresponding to the following GitHub usernames: This might be simply due to a missing "GitHub Name" entry in one's b.p.o account settings. This is necessary for legal reasons before we can look at this contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. CLA MissingOur records indicate the following people have not signed the CLA: For legal reasons we need all the people listed to sign the CLA before we can look at your contribution. Please follow the steps outlined in the CPython devguide to rectify this issue. If you have recently signed the CLA, please wait at least one business day You can check yourself to see if the CLA has been received. Thanks again for the contribution, we look forward to reviewing it! |
|
I have only just signed the CLA myself so I guess that hasn't kicked in yet. |
This patch was originally contributed by Daniel Urban, whose summary follows: > I'm attaching a patch implementing super.__setattr__ (and > __delattr__). > The implementation in the patch only works, if super can find a data > descriptor in the MRO, otherwise it throws an AttributeError. As it > can be seen in the tests, in some cases this may result in > counter-intuitive behaviour. But I wasn't able to find another > behaviour, that is consistent with both super.__getattr__ and normal > __setattr__ semantics. Co-authored-by: hab <[email protected]> Co-authored-by: Markus Kitsinger (SwooshyCueb) <[email protected]>
SwooshyCueb
force-pushed
the
14965.main
branch
from
356b598 to
b955e79
Compare
|
This PR is stale because it has been open for 30 days with no activity. |
|
hmm.. is there a bot issue, or..? |
github-actions
bot
removed
the
stale
|
Checking the CLAs at https://check-python-cla.herokuapp.com/, SwooshyCueb is good:
And so is habnabit:
Leaving @durban "does not have bpo account 🤷🏻":
@durban Please can you log in to https://bugs.python.org -> Your details (on the left) -> add "durban" to GitHub Name -> then recheck at https://check-python-cla.herokuapp.com/ ? |
|
@hugovk Ok, done. |
|
Thanks! All good now!
|
|
@gvanrossum Is this something you want to have done? Your original design for super intentionally supported only named method calls instead of operator-to-magic-method calls. Also, I think it is a questionable practice (violation of the open/closed principle, encapsulation, and separation-of-concerns) for a subclass to be mutating the attributes of a parent class. This is doubly true for Python's version of super() where the caller can't know it advance where the call is going to land because the MRO is determined dynamically based on the instance. |
|
I think this is fine -- I didn't read the C code but I read the tests and they make sense. After all a property is just an alternative way to call |
| Py_INCREF(descr); | ||
| f = Py_TYPE(descr)->tp_descr_set; | ||
| if ((f != NULL) && PyDescr_IsData(descr)) { | ||
| /* We found a data descriptor: */ |
There was a problem hiding this comment.
Seems like the behavior if there isn't a data descriptor is that we fall back to PyObject_GenericSetAttr, which will fail because super() doesn't allow setting any attributes. Shouldn't we instead set the attr on the object super is proxying for?
Use case to think about: We're using super().x = ... to set a @property on a base class. Then the base class is refactored so that x is a regular attribute, not a @property. As a user, I would expect super().x = ... to continue to work as before.
I'm resubmitting #26194 with proper commit authorship, as the PR has gone stale and this seems to be the only thing blocking the PR from being merged.
Both the original submitter (@habnabit) and the patch author (@durban) have signed the CLA.
It is the opinion of the original submitter (@habnabit) that this PR should be backported, as it is arguably a bugfix. I do not disagree.
This patch was originally contributed by Daniel Urban, whose summary
follows:
https://bugs.python.org/issue14965