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

Abstract property setter/deleter implementation not enforced, but documented as such #83888

Open
arnvollebregtkpn mannequin opened this issue Feb 21, 2020 · 5 comments
Open
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error

Comments

@arnvollebregtkpn
Copy link
Mannequin

arnvollebregtkpn mannequin commented Feb 21, 2020

BPO 39707
Nosy @gvanrossum, @rhettinger, @MojoVampire

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 2020-02-21.10:59:10.737>
labels = ['type-bug', '3.8', '3.9', '3.10', '3.7', 'docs']
title = 'Abstract property setter/deleter implementation not enforced, but documented as such'
updated_at = <Date 2020-12-09.22:45:34.731>
user = 'https://bugs.python.org/arnvollebregtkpn'

bugs.python.org fields:

activity = <Date 2020-12-09.22:45:34.731>
actor = 'gvanrossum'
assignee = 'docs@python'
closed = False
closed_date = None
closer = None
components = ['Documentation']
creation = <Date 2020-02-21.10:59:10.737>
creator = 'arn.vollebregt.kpn'
dependencies = []
files = []
hgrepos = []
issue_num = 39707
keywords = []
message_count = 5.0
messages = ['362403', '362413', '362438', '382810', '382811']
nosy_count = 5.0
nosy_names = ['gvanrossum', 'rhettinger', 'docs@python', 'josh.r', 'arn.vollebregt.kpn']
pr_nums = []
priority = 'normal'
resolution = None
stage = 'resolved'
status = 'open'
superseder = None
type = 'behavior'
url = 'https://bugs.python.org/issue39707'
versions = ['Python 3.7', 'Python 3.8', 'Python 3.9', 'Python 3.10']

@arnvollebregtkpn
Copy link
Mannequin Author

arnvollebregtkpn mannequin commented Feb 21, 2020

When concretely implementing an abstract ABC class with an abstract property getter, setter and deleter it is not enfored that the setter and deleter are implemented. Instead, the property is treated as a read-only property (as would normally be the case without a setter/deleter definition for a property) and the setter/deleter code from the abstract class is not present in the child class.

I would expect a TypeError exception when an abstract property is defined with a getter, setter and deleter but only the getter is implemented in a subclass (as is the case when not implementing the property getter). As a fallback, I would find it acceptable the code from the abstract class to be present in the child class, so at least the code that is defined there (in this case raising a NotImplementedError exception) would be executed.

An interactive interpreter session to replicate this behavior:

arn@hacktop:~$ python3
Python 3.7.5 (default, Nov 20 2019, 09:21:52)
[GCC 9.2.1 20191008] on linux
Type "help", "copyright", "credits" or "license" for more information.
>>> import abc
>>>
>>> # Define the (abstract) interface.
... class MyInterface(abc.ABC):
...
...     # Property getter.
...     @property
...     @abc.abstractmethod
...     def myProperty(self) -> str:
...         raise NotImplementedError
...
...     # Property setter.
...     @myProperty.setter
...     @abc.abstractmethod
...     def myProperty(self, value: str) -> None:
...         raise NotImplementedError
...
...     # Property deleter.
...     @myProperty.deleter
...     @abc.abstractmethod
...     def myProperty(self) -> None:
...         raise NotImplementedError
...
>>> # Implemented the interface.
... class MyImplementation(MyInterface):
...     
...     # No abstract method implementation(s).
...     pass
...
>>> # Creation of MyImplementation object raises TypeError as expected.
... obj = MyImplementation()
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
TypeError: Can't instantiate abstract class MyImplementation with abstract methods myProperty
>>> import dis
>>> # The property getter code would raise an exception as defined in MyInterface.
... dis.dis(MyImplementation.myProperty.fget.__code__.co_code)
          0 LOAD_GLOBAL              0 (0)
          2 RAISE_VARARGS            1
          4 LOAD_CONST               0 (0)
          6 RETURN_VALUE
>>> # The property setter code would raise an exception as defined in MyInterface.
... dis.dis(MyImplementation.myProperty.fset.__code__.co_code)
          0 LOAD_GLOBAL              0 (0)
          2 RAISE_VARARGS            1
          4 LOAD_CONST               0 (0)
          6 RETURN_VALUE
>>> # The property deleter code would raise an exception as defined in MyInterface.
... dis.dis(MyImplementation.myProperty.fdel.__code__.co_code)
          0 LOAD_GLOBAL              0 (0)
          2 RAISE_VARARGS            1
          4 LOAD_CONST               0 (0)
          6 RETURN_VALUE
>>> # Let's reimplement with only the property getter.
... class MyImplementation(MyInterface):
...
...     # Only implement abstract property getter.
...     @property
...     def myProperty(self) -> str:
...         return "foobar"
...
>>> # Object can be created (against expectations).
... obj = MyImplementation()
>>> # The property getter works as defined.
... obj.myProperty
'foobar'
>>> # The property cannot be set (read-only).
... obj.myProperty = "barfoo"
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
AttributeError: can't set attribute
>>> # The property cannot be deleted (read-only).
... del obj.myProperty
Traceback (most recent call last):
  File "<stdin>", line 2, in <module>
AttributeError: can't delete attribute
>>> # The property getter code returns a string as defined in MyImplementation.
... type(MyImplementation.myProperty.fget)
<class 'function'>
>>> dis.dis(MyImplementation.myProperty.fget.__code__.co_code)
          0 LOAD_CONST               1 (1)
          2 RETURN_VALUE
>>> # The property setter code however does not exist, although defined in MyInterface.
... type(MyImplementation.myProperty.fset)
<class 'NoneType'>
>>> # Nor does the property deleter code, although defined in MyInterface.
... type(MyImplementation.myProperty.fdel)
<class 'NoneType'>

@arnvollebregtkpn arnvollebregtkpn mannequin added 3.7 (EOL) end of life type-bug An unexpected behavior, bug, or error labels Feb 21, 2020
@rhettinger
Copy link
Contributor

Off-hand, I don't see how this can be easily fixed because the setters and deleters are all part of a single property object.

When the subclass defines a property without a getter and setter, the inherited abstract property (that does have a getter and setter) is masked.

Right now, ABCMeta only looks at the concrete property. It would have to be modified to scan the next in MRO for an abstract property and the pick apart its component fget, fset, and fdel. That would be a significant jump in complexity with only a minimal payoff.

@gvanrossum
Copy link
Member

I agree with Raymond here. This is a job for a static type checker like mypy. Closing.

@MojoVampire
Copy link
Mannequin

MojoVampire mannequin commented Dec 9, 2020

If this is going to be closed as rejected, I think it still needs some improvement to the documentation. Right now, the docs for abstractproperty (deprecated in favor of combining property and abstractmethod) state:

"If only some components are abstract, only those components need to be updated to create a concrete property in a subclass:"

This heavily implies that if *all* components of the property are abstract, they must *all* be updated to create a concrete property on the subclass, when that is not the case (it's documenting a special way of overriding just one component by borrowing the base class, not a normal means of defining a property). If nothing else, mentioning this quirk in the docs seems like it would save confusion (e.g. https://stackoverflow.com/questions/65224767/python-abstract-property-cant-instantiate-abstract-class-with-abstract-me ).

@MojoVampire MojoVampire mannequin added 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir labels Dec 9, 2020
@MojoVampire MojoVampire mannequin reopened this Dec 9, 2020
@MojoVampire MojoVampire mannequin changed the title Abstract property setter/deleter implementation not enforced. Abstract property setter/deleter implementation not enforced, but documented as such Dec 9, 2020
@MojoVampire MojoVampire mannequin assigned docspython Dec 9, 2020
@MojoVampire MojoVampire mannequin added the 3.8 (EOL) end of life label Dec 9, 2020
@gvanrossum
Copy link
Member

Josh, feel free to submit a PR (make sure it mentions this issue).

@ezio-melotti ezio-melotti transferred this issue from another repository Apr 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3.7 (EOL) end of life 3.8 (EOL) end of life 3.9 only security fixes 3.10 only security fixes docs Documentation in the Doc dir type-bug An unexpected behavior, bug, or error
Projects
None yet
Development

No branches or pull requests

2 participants