Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

super() and property inheritance behavior #59170

Open
ghost opened this issue May 31, 2012 · 21 comments
Open

super() and property inheritance behavior #59170

ghost opened this issue May 31, 2012 · 21 comments
Labels
3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error

Comments

@ghost
Copy link

ghost commented May 31, 2012

BPO 14965
Nosy @rhettinger, @ncoghlan, @tiran, @merwok, @alex, @asvetlov, @durban, @Bluehorn, @ethanfurman, @kynan, @RonnyPfannschmidt, @habnabit, @kenodegard, @willrazen, @SwooshyCueb
PRs
  • bpo-14965: Proxy super().x = y and del super().x #26194
  • bpo-14965: Proxy super().x = y and del super().x (updated) #29950
  • Files
  • super_setattr.patch: setattr and delattr for super
  • 44560_44559.diff: Alternate patch, targeted at Python 2.7
  • superprop.py
  • duper.py: Python workaround
  • Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.

    Show more details

    GitHub fields:

    assignee = None
    closed_at = None
    created_at = <Date 2012-05-31.03:04:00.947>
    labels = ['interpreter-core', 'type-bug', '3.11']
    title = 'super() and property inheritance behavior'
    updated_at = <Date 2021-12-09.19:32:05.455>
    user = None

    bugs.python.org fields:

    activity = <Date 2021-12-09.19:32:05.455>
    actor = 'Ronny.Pfannschmidt'
    assignee = 'none'
    closed = False
    closed_date = None
    closer = None
    components = ['Interpreter Core']
    creation = <Date 2012-05-31.03:04:00.947>
    creator = '\xe7\x8c\xab.\xe9\xbb\x92'
    dependencies = []
    files = ['25820', '26966', '37546', '50057']
    hgrepos = []
    issue_num = 14965
    keywords = ['patch']
    message_count = 21.0
    messages = ['161980', '162050', '162283', '168895', '174404', '174863', '174911', '179217', '232438', '233127', '275855', '391838', '393838', '394071', '394074', '394094', '394657', '408070', '408071', '408072', '408155']
    nosy_count = 25.0
    nosy_names = ['rhettinger', 'ncoghlan', 'christian.heimes', 'habnabit', 'eric.araujo', 'alex', 'cvrebert', 'asvetlov', 'THRlWiTi', 'daniel.urban', 'dabeaz', 'torsten', 'ethan.furman', 'kynan', 'josmiley', 'Ronny.Pfannschmidt', 'piotr.dobrogost', '\xe7\x8c\xab.\xe9\xbb\x92', 'simonzack', 'jcasale', 'Aaron Gallagher', 'kenodegard', 'Victor Milovanov', 'willrazen', 'SwooshyCueb']
    pr_nums = ['26194', '29950']
    priority = 'normal'
    resolution = None
    stage = 'patch review'
    status = 'open'
    superseder = None
    type = 'behavior'
    url = 'https://bugs.python.org/issue14965'
    versions = ['Python 3.11']

    @ghost
    Copy link
    Author

    ghost commented May 31, 2012

    super() objects allow access to inherited properties fget() but not fset() or fdel(), resulting in unexpected behavior.

    Today on pydev thread 'Property inheritance in Python' GvR said "I
    don't see the need for a Python-Ideas detour. It seems worth fixing"

    >>> class BaseProp(object):
    ...     @property
    ...     def p(self):
    ...         return self._p
    ...     @p.setter
    ...     def p(self, value):
    ...         self._p = value
    >>> class DerivedProp(BaseProp):
    ...     @property
    ...     def p(self):
    ...         return super(DerivedProp, self).p * 2
    ...     @p.setter
    ...     def p(self, value):
    ...         super(DerivedProp, self).p = value / 2
    >>> d = DerivedProp()
    >>> d._p = 21
    >>> d.p
    42
    >>> d.p = 50
    Traceback (most recent call last):
       ...
    AttributeError: 'super' object has no attribute 'p'

    Repository owner added stdlib Python modules in the Lib dir type-bug An unexpected behavior, bug, or error labels May 31, 2012
    @josmiley
    Copy link
    Mannequin

    josmiley mannequin commented Jun 1, 2012

    >>> class DerivedProp(BaseProp):
    ...     @property
    ...     def p(self):
    ...         return super(DerivedProp, self).p * 2
    ...     @p.setter
    ...     def p(self, value):
    ...         BaseProp.p.__set__(self,value / 2)

    @durban
    Copy link
    Mannequin

    durban mannequin commented Jun 4, 2012

    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.

    @durban durban mannequin added extension-modules C modules in the Modules dir and removed stdlib Python modules in the Lib dir labels Jun 4, 2012
    @Bluehorn
    Copy link
    Mannequin

    Bluehorn mannequin commented Aug 22, 2012

    I stumbled across this omission as well in 2010 and brought this up on python-dev:

    http://mail.python.org/pipermail/python-dev/2010-April/099672.html

    There were no replies, but perhaps my post adds a bit of information and also there is another patch linked from there. I attached my patch from 2010 for reference.

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Nov 1, 2012

    I'm -0 for proposed changes, these changes reduce code readability from my perspective.
    I think better to use existing approach: explicitly specify what do you want to do with overloaded properties.

    @ghost
    Copy link
    Author

    ghost commented Nov 5, 2012

    I'm not a python dev, but would you say

    super(self.__class__, self.__class__).x.fset(self, value)

    is more readable than

    super().x = value

    @asvetlov
    Copy link
    Contributor

    asvetlov commented Nov 5, 2012

    I would say

    @x.deleter
    def x(self):
        del super().x

    confuses me a bit.
    But I'm only -0, let's see other developers for their opinions.

    @dabeaz
    Copy link
    Mannequin

    dabeaz mannequin commented Jan 6, 2013

    Just as a note, there is a distinct possibility that a "property" in a superclass could be some other kind of descriptor object that's not a property. To handle that case, the solution of

    super(self.__class__, self.__class__).x.fset(self, value)

    would actually have to be rewritten as

    super(self.__class__, self.__class__).x.__set__(self, value)

    That said, I agree it would be nice to have a simplified means of accomplishing this.

    @simonzack
    Copy link
    Mannequin

    simonzack mannequin commented Dec 10, 2014

    +1 to this feature, this will dramatically simplify property setting code.

    @simonzack
    Copy link
    Mannequin

    simonzack mannequin commented Dec 27, 2014

    For those who want to use this right away, I've added a python implementation of the patch, which passes the unit tests. There's a slight difference in usage, where instead of using super() directly, super_prop(super()) needs to be used, so we can still use super without arguments.

    @tiran
    Copy link
    Member

    tiran commented Sep 11, 2016

    I had to add a workaround to ssl.SSLContext and would appreciate a better solution.

    @Mariatta Mariatta added the 3.7 (EOL) end of life label Mar 23, 2017
    @VictorMilovanov
    Copy link
    Mannequin

    VictorMilovanov mannequin commented Apr 25, 2021

    There's a patch attached to this bug. Why is its stage "needs patch"?

    @wyz23x2 wyz23x2 mannequin added interpreter-core (Objects, Python, Grammar, and Parser dirs) 3.9 only security fixes 3.10 only security fixes 3.11 only security fixes and removed extension-modules C modules in the Modules dir 3.7 (EOL) end of life labels May 5, 2021
    @habnabit
    Copy link
    Mannequin

    habnabit mannequin commented May 17, 2021

    @daniel.urban I'm attempting to move this patch along, but since the contributing process has changed in the years since your patch, you'll need to sign the CLA. Are you interested in picking this back up at all? I haven't been given any indication of how to proceed if I'm doing this on your behalf, but hopefully the core team will enlighten us.

    @willrazen
    Copy link
    Mannequin

    willrazen mannequin commented May 20, 2021

    @simonzack Your superprop.py doesn't work for multiple inheritance, because you're using __thisclass__.__mro__ in each step instead of the initial object mro

    @durban
    Copy link
    Mannequin

    durban mannequin commented May 20, 2021

    @habnabit I believe I've already signed some contributor form some years ago. If there is a new one, I can sign that one too.

    @willrazen
    Copy link
    Mannequin

    willrazen mannequin commented May 21, 2021

    Fixed superprop.py workaround, now works with multiple inheritance and follows mro adequately. Renamed to duper.py as inspired by Torsten. Uploading here and also to https://gist.github.com/willrazen/bef3fcb26a83dffb6692e5e10d3e67ac

    @habnabit
    Copy link
    Mannequin

    habnabit mannequin commented May 28, 2021

    @daniel.urban would you kindly resubmit your patch as a PR to the cpython repo? I've learned out-of-band from someone else that putting patches on bpo is considered obsolete. you can use the PR I've submitted (#26194) and reset the author.

    I'd be happy to do it myself (giving you a branch that's all set up, so all you need to do is click the 'new PR' button) if you tell me what to set the author to.

    @rhettinger
    Copy link
    Contributor

    -0 from me as well. I don't think this is common or something that should be encouraged. As Andrew points out, "del super().x" doesn't have an obvious meaning and it could be regarded as a code smell.

    The OP's first example would be an unpleasant API to debug -- it exhibits tight coupling between the parent and child class, it has Liskov issues, and it has implicit forwarding and indirection through descriptors. The tight coupling is especially problematic because Python's super() isn't guaranteed to call the parent class; rather, it can call a sibling class as determined by the MRO which cannot be known at the time the class is written.

    Another thought is that super() is intentionally not a completely transparent proxy. While an explicit call super().__getitem__(k) works, we've denied support for super()[k]. To me, "super().x = 10" and "del super().x" fall in the same category.

    Looking at the OP's

    Fortunately, it doesn't seem to be a common need to use super() in a property setter or deleter to bypass the current class and call setter or deleter in a parent class property. Arguably, this kind of tight coupling isn't good design. The OP's first example would be an unpleasant API to debug.

    FWIW,

    @rhettinger rhettinger removed the 3.9 only security fixes label Dec 9, 2021
    @rhettinger rhettinger removed the 3.10 only security fixes label Dec 9, 2021
    @rhettinger
    Copy link
    Contributor

    Another thought: Given that this tracker issue has been open for a decade without resolution, we have evidence that this isn't an important problem in practice.

    Arguably, people have been better off being nudged in another direction toward better design or having been forced to be explicit about what method is called and when.

    @habnabit
    Copy link
    Mannequin

    habnabit mannequin commented Dec 9, 2021

    I will note, Raymond, that I’ve wanted this for years before discovering
    this bpo issue, and I found it because you linked it on Twitter. ;)

    On Wed, Dec 8, 2021 at 19:08 Raymond Hettinger <report@bugs.python.org>
    wrote:

    Raymond Hettinger <raymond.hettinger@gmail.com> added the comment:

    Another thought: Given that this tracker issue has been open for a decade
    without resolution, we have evidence that this isn't an important problem
    in practice.

    Arguably, people have been better off being nudged in another direction
    toward better design or having been forced to be explicit about what method
    is called and when.

    ----------


    Python tracker <report@bugs.python.org>
    <https://bugs.python.org/issue14965\>


    @RonnyPfannschmidt
    Copy link
    Mannequin

    RonnyPfannschmidt mannequin commented Dec 9, 2021

    im on the noisy list because i faced this first in 2012

    a key problem where i ran into this was mixins, - depending on whether a mixin was added or not one would get errors or not

    from my pov a super object should look like the "next class"

    so properties of the next base should behave as such, special methods as well

    Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
    Labels
    3.11 only security fixes interpreter-core (Objects, Python, Grammar, and Parser dirs) type-bug An unexpected behavior, bug, or error
    Projects
    None yet
    Development

    No branches or pull requests

    5 participants