@Override decorator
Python Ideas, While working on some object-oriented projects in Java, I noticed that Python does not have an @override decorator for methods that are derived from a parent class but overridden for the unique purposes of a base class. With the creation of the @abstractmethod decorator, the override decorator could follow in clearly distinguishing the logic and design between parent/base classes. Why I would think an override decorator might be useful: For other people reading the code, it would be great to distinguish between methods that are unique to a class and methods that are inherited from a parent class. Not all methods inherited might be overridden, so keeping track of which inherited methods are overridden and which are not would be nice. With the advent of static typing from mypy: Having the decorator could corroborate the fact that the given method overrides the parent method correctly (correct name + parameter list). When the parent class changes, such as the name or parameter list of an abstract method, the children classes should be updated as well. mypy could easily target the methods that need to be altered with the correct method signature. If you don’t have an override decorator and overrode a parent method, then there could be some error complaining about this. This would be extremely useful to prevent accidental errors. There is some interest for this as expressed on Stack Overflow (http://stackoverflow.com/questions/1167617/in-python-how-do-i-indicate-im-ov... <http://stackoverflow.com/questions/1167617/in-python-how-do-i-indicate-im-ov...>) and some people have also made packages for this, but having it in the standard distribution would be nice. Thoughts? Sincerely, Shrey Desai https://github.com/shreydesai <https://github.com/shreydesai>
It could also be useful to have @override decorator functionality in zope.interface http://zopeinterface.readthedocs.io/en/latest/verify.html zope.interface supports interface verification at test and runtime. AAIU, @override would raise an Exception: - a) when a subclass specifies @override for a method not defined in a superclass - b) when the argspecs are different AFAIU, a)-like functionality does not yet exist in zope.interface On Jul 18, 2016 1:45 PM, "Shrey Desai" <shreydesai@me.com> wrote:
On 07/18/2016 09:44 AM, Shrey Desai wrote:
While working on some object-oriented projects in Java, I noticed that Python does not have an *@override* decorator for methods that are derived from a parent class but overridden for the unique purposes of a base class. With the creation of the *@abstractmethod* decorator, the override decorator could follow in clearly distinguishing the logic and design between parent/base classes.
I certainly see the usefulness of such a decorator, but I don't think it needs to be in stdlib. -- ~Ethan~
I would really like this as I currently comment all my overridden methods to remind myself what is an override. However, I suggest two overrides decorators: one for pure virtuals, and one where the pattern is call super. The assertion for argspecs should only apply when the superclass has a pure virtual method. The other pattern is with multiple inheritance where it's common for subclasses to pull off the arguments they need and forward the rest to the superclass. In that case, it would be nice to check (as best as possible, by for example walking the parsed code) that the member calls super. A common bug is to forget to do this, which might not be detected until multiple inheritance causes the mro to change the superclass, and the new superclass has important behavior that's not being run. Best, Neil On Monday, July 18, 2016 at 1:45:40 PM UTC-4, Shrey Desai wrote:
Terry Reedy <tjreedy@udel.edu> writes:
The primary behaviour I've seen for such suggestions is that ‘override’ will verify the method is already defined in the specified base class (or, if no class specified, then in at least one of the base classes), and if not will raise AssertionError. The proposal describes the behaviour this way:
-- \ “Holy polar ice sheet, Batman!” —Robin | `\ | _o__) | Ben Finney
On Mon, Jul 18, 2016 at 09:44:45AM -0700, Shrey Desai wrote:
I fail to see that this is a sufficiently great problem to require a decorator. Occasionally I've added a comment to a method "Overriding mixin" or "Overriding class XXX" for example, but mostly I don't even bother. It's either obvious or unimportant or both. And when I do find myself writing such a comment, that's a good sign that my class hierarchy is too hard to understand and needs to be refactored.
There is no requirement that the subclass method uses the same parameter names, or even signature, as the superclass method.
I don't see it being very useful. Perhaps for people who have learned bad habits from Java and write deep class hierarchies, but we shouldn't encourage such anti-patterns. If people wish to write their own @override decorator, they should go right ahead, but I don't think it belongs in the standard library. -- Steve
The same argument would apply against abstractmethod... I have definitely wondered when looking over some code whether it was defining or overriding a method. But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe? On Monday, July 18, 2016, Steven D'Aprano <steve@pearwood.info> wrote:
-- --Guido (mobile)
On 07/18/2016 09:04 PM, Guido van Rossum wrote:
The same argument would apply against abstractmethod... I have definitely wondered when looking over some code whether it was defining or overriding a method. But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe?
Do you mean something like: def __new__(metacls, cls, bases, clsdict): for name, attr in clsdict.items(): if callable(attr): is_override = isinstance(attr, override) # search for name in bases # if found and is_override is False, raise # or if not found and is_override is True, raise -- ~Ethan~
On 19 July 2016 at 14:04, Guido van Rossum <guido@python.org> wrote:
This is trickier in Python than it is in Java, since we support multiple inheritance, and we don't require that subclasses be drop-in replacements for their parent class. The key difference I see between @abstractmethod and this proposal is that @abstractmethod is a service that base class authors provide to subclass authors to say "Hey, when subclassing this, you *need* to override these, or your subclass won't work properly". In effect, it's an implicit test case for all future subclass definitions. By contrast, this proposal would go in the other direction: it would be an assertion by the author of a *subclass* that that particular method isn't new, it's inherited from a parent class, and they're replacing the existing implementation with a new one. It would only add new information for readers in cases where the method wasn't already calling super() (since the latter already implies you expect there to be at least one further implementation along the MRO to call). In my view, that puts it more in the category of type annotations and the "missing attribute" detection in tools like pylint than it does abstractmethod: rather than checking it at runtime, you'd want static checkers like mypy to flag it as a structural error if a method declared as replacing a base class method wasn't actually doing so. So a "typing.override" to complement "typing.overload" would make sense to me, but a checked-at-runtime decorator wouldn't (doing it that way would also let you put the override annotation in stub files instead of directly in the code). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Tue, Jul 19, 2016 at 1:19 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
Why can't @overrides be processed at definition time? It could check one of two cases: either you're replacing an abstractmethod, in which case the parameter lists should be the same; or else, you're overriding a base class method in which case, you should probably call super. Finally, it could prepend the __doc__ attribute from the parent class method if there is one to the doc attribute on this method if there is one.
On 19 July 2016 at 16:44, Neil Girdhar <mistersheik@gmail.com> wrote:
Why can't @overrides be processed at definition time?
I didn't say it can't, I said I think it shouldn't be, and it shouldn't be for the same reason we don't process type annotations at runtime: this proposal falls into the realm of gradual typing, and gradual typing is an *optional* behaviour for use on projects that are sufficiently large with high enough personnel churn to see the benefits that outweigh the cost in boilerplate. With PEP 484 and typing, we've already established the precedent that people shouldn't go rushing off to their favourite projects and submit low value PRs that just add annotations for the sake of adding annotations. The proposed decorator here is a different kind of semantic annotation, but it's still one that should only be added to a project if the existing contributors to that project think it will help them maintain the code. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Tue, Jul 19, 2016 at 10:42 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
The problem I have with the type annotation behavior that you're proposing is that you lose the definition-time assertions that overrides could raise. Even in a small project like mine (20k lines), that would be a big asset for me to eliminate a whole class of bugs. Your second point about people submitting PRs whereby they add decorators to everything shouldn't be an argument against new features. No one is forced to use @overrides, and projects can decide for themselves whether want to use it.
On Tue, Jul 19, 2016, 9:51 AM Neil Girdhar <mistersheik@gmail.com> wrote:
Could you explain that further? Are you saying you need an override check for dynamically created/modified parent classes such that static analysis would not suffice? Your second point about people submitting PRs whereby they add decorators
You're right (and so is Nick): the static analysis would suffice. I guess you don't get the automatic docstring copying. My personal feeling is that it is also nice to have (even if I don't use it) support for dynamically created/modified parent classes. Best, Neil On Tue, Jul 19, 2016 at 11:32 AM Michael Selik <michael.selik@gmail.com> wrote:
On Jul 19, 2016 10:43 AM, "Nick Coghlan" <ncoghlan@gmail.com> wrote:
Is there enough information, in all cases, to do the check at definition time? - Obviously this wouldnt work for AOP-style dynamic mixins which are/could be modified by changing {__bases__, __class__, ...?} at runtime (certainly an infrequent use case and a different style of composition than @override-relevant inheritance) - __new__
What reasons are there for @override to be in stdlib?: - no import required (though it would still be necessary for backwards compatibility) - cross-cutting concerns - definition-time type verificayions - runtime type verifications aside: Spring Python may be of interest to people more familiar with Java
On Tue, Jul 19, 2016, 12:08 PM Wes Turner <wes.turner@gmail.com> wrote:
This is an argument for adding this feature to the typing module as opposed to a decorator, correct? Changing the base dynamically would not trigger another call to @overrides, even with some fancy metaclassing. The check wouldn't occur until instantiation or calling the method. Going for instantiation-time checks could get awkward via metaclass. To avoid conflicts I guess OverrideMeta would be an extension of ABCMeta. If just using __new__ it's too easy to accidentally bypass. Call-time checks would add overhead to every method call.
On Tue, Jul 19, 2016 at 03:18:34PM +1000, Nick Coghlan wrote:
On 19 July 2016 at 14:04, Guido van Rossum <guido@python.org> wrote in response to my comments:
The same argument would apply against abstractmethod...
Well, to be honest, I've never been in a situation where I either needed abstractmethod or knowingly had to deal with a class that used it. Maybe I just don't use classes enough to see why this proposal is useful.
Isn't that one of the features of IDEs? (Which doesn't help those who aren't using an IDE.)
But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe?
[Nick]
Indeed.
Certainly this is something which static checkers *could* do, but surely a runtime check would be sufficient? The runtime check only needs to occur when the class is defined, not on every call. It could look something like this: def override(method): proxy = get_class_proxy() # MAGIC GOES HERE assert hasattr(method.__name__, proxy) return method I say MAGIC GOES HERE because of course the class doesn't exist yet. The get_class_proxy() function should return some sort of proxy to the soon-to-exist class' MRO. (Maybe this is harder to write than I thought.) Having it work at class definition time rather than compile-time means that it will work with odd corner cases like injected methods: class C: pass ... ... C.method = something() class D(C): @override def method(self): pass I wouldn't expect a purely static checker to necessarily realise that C.method exists, but there's no problem for the runtime check.
-- Steve
On Tue, Jul 19, 2016 at 7:51 AM Steven D'Aprano <steve@pearwood.info> wrote:
(Maybe this is harder to write than I thought.)
I don't think you need any magic. You can either do it like @abstractmethod whereby you put the logic in ABCMeta or given that PEP 487 has been accepted, put the logic in a plain old base class (ABCBase?) that has the logic in __init_subclass__. This would work with your corner case. It also is a bit more flexible. Having it work at class definition time rather than compile-time means
It could also be useful to have @override decorator functionality in zope.interface http://zopeinterface.readthedocs.io/en/latest/verify.html zope.interface supports interface verification at test and runtime. AAIU, @override would raise an Exception: - a) when a subclass specifies @override for a method not defined in a superclass - b) when the argspecs are different AFAIU, a)-like functionality does not yet exist in zope.interface On Jul 18, 2016 1:45 PM, "Shrey Desai" <shreydesai@me.com> wrote:
On 07/18/2016 09:44 AM, Shrey Desai wrote:
While working on some object-oriented projects in Java, I noticed that Python does not have an *@override* decorator for methods that are derived from a parent class but overridden for the unique purposes of a base class. With the creation of the *@abstractmethod* decorator, the override decorator could follow in clearly distinguishing the logic and design between parent/base classes.
I certainly see the usefulness of such a decorator, but I don't think it needs to be in stdlib. -- ~Ethan~
I would really like this as I currently comment all my overridden methods to remind myself what is an override. However, I suggest two overrides decorators: one for pure virtuals, and one where the pattern is call super. The assertion for argspecs should only apply when the superclass has a pure virtual method. The other pattern is with multiple inheritance where it's common for subclasses to pull off the arguments they need and forward the rest to the superclass. In that case, it would be nice to check (as best as possible, by for example walking the parsed code) that the member calls super. A common bug is to forget to do this, which might not be detected until multiple inheritance causes the mro to change the superclass, and the new superclass has important behavior that's not being run. Best, Neil On Monday, July 18, 2016 at 1:45:40 PM UTC-4, Shrey Desai wrote:
Terry Reedy <tjreedy@udel.edu> writes:
The primary behaviour I've seen for such suggestions is that ‘override’ will verify the method is already defined in the specified base class (or, if no class specified, then in at least one of the base classes), and if not will raise AssertionError. The proposal describes the behaviour this way:
-- \ “Holy polar ice sheet, Batman!” —Robin | `\ | _o__) | Ben Finney
On Mon, Jul 18, 2016 at 09:44:45AM -0700, Shrey Desai wrote:
I fail to see that this is a sufficiently great problem to require a decorator. Occasionally I've added a comment to a method "Overriding mixin" or "Overriding class XXX" for example, but mostly I don't even bother. It's either obvious or unimportant or both. And when I do find myself writing such a comment, that's a good sign that my class hierarchy is too hard to understand and needs to be refactored.
There is no requirement that the subclass method uses the same parameter names, or even signature, as the superclass method.
I don't see it being very useful. Perhaps for people who have learned bad habits from Java and write deep class hierarchies, but we shouldn't encourage such anti-patterns. If people wish to write their own @override decorator, they should go right ahead, but I don't think it belongs in the standard library. -- Steve
The same argument would apply against abstractmethod... I have definitely wondered when looking over some code whether it was defining or overriding a method. But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe? On Monday, July 18, 2016, Steven D'Aprano <steve@pearwood.info> wrote:
-- --Guido (mobile)
On 07/18/2016 09:04 PM, Guido van Rossum wrote:
The same argument would apply against abstractmethod... I have definitely wondered when looking over some code whether it was defining or overriding a method. But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe?
Do you mean something like: def __new__(metacls, cls, bases, clsdict): for name, attr in clsdict.items(): if callable(attr): is_override = isinstance(attr, override) # search for name in bases # if found and is_override is False, raise # or if not found and is_override is True, raise -- ~Ethan~
On 19 July 2016 at 14:04, Guido van Rossum <guido@python.org> wrote:
This is trickier in Python than it is in Java, since we support multiple inheritance, and we don't require that subclasses be drop-in replacements for their parent class. The key difference I see between @abstractmethod and this proposal is that @abstractmethod is a service that base class authors provide to subclass authors to say "Hey, when subclassing this, you *need* to override these, or your subclass won't work properly". In effect, it's an implicit test case for all future subclass definitions. By contrast, this proposal would go in the other direction: it would be an assertion by the author of a *subclass* that that particular method isn't new, it's inherited from a parent class, and they're replacing the existing implementation with a new one. It would only add new information for readers in cases where the method wasn't already calling super() (since the latter already implies you expect there to be at least one further implementation along the MRO to call). In my view, that puts it more in the category of type annotations and the "missing attribute" detection in tools like pylint than it does abstractmethod: rather than checking it at runtime, you'd want static checkers like mypy to flag it as a structural error if a method declared as replacing a base class method wasn't actually doing so. So a "typing.override" to complement "typing.overload" would make sense to me, but a checked-at-runtime decorator wouldn't (doing it that way would also let you put the override annotation in stub files instead of directly in the code). Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Tue, Jul 19, 2016 at 1:19 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
Why can't @overrides be processed at definition time? It could check one of two cases: either you're replacing an abstractmethod, in which case the parameter lists should be the same; or else, you're overriding a base class method in which case, you should probably call super. Finally, it could prepend the __doc__ attribute from the parent class method if there is one to the doc attribute on this method if there is one.
On 19 July 2016 at 16:44, Neil Girdhar <mistersheik@gmail.com> wrote:
Why can't @overrides be processed at definition time?
I didn't say it can't, I said I think it shouldn't be, and it shouldn't be for the same reason we don't process type annotations at runtime: this proposal falls into the realm of gradual typing, and gradual typing is an *optional* behaviour for use on projects that are sufficiently large with high enough personnel churn to see the benefits that outweigh the cost in boilerplate. With PEP 484 and typing, we've already established the precedent that people shouldn't go rushing off to their favourite projects and submit low value PRs that just add annotations for the sake of adding annotations. The proposed decorator here is a different kind of semantic annotation, but it's still one that should only be added to a project if the existing contributors to that project think it will help them maintain the code. Cheers, Nick. -- Nick Coghlan | ncoghlan@gmail.com | Brisbane, Australia
On Tue, Jul 19, 2016 at 10:42 AM Nick Coghlan <ncoghlan@gmail.com> wrote:
The problem I have with the type annotation behavior that you're proposing is that you lose the definition-time assertions that overrides could raise. Even in a small project like mine (20k lines), that would be a big asset for me to eliminate a whole class of bugs. Your second point about people submitting PRs whereby they add decorators to everything shouldn't be an argument against new features. No one is forced to use @overrides, and projects can decide for themselves whether want to use it.
On Tue, Jul 19, 2016, 9:51 AM Neil Girdhar <mistersheik@gmail.com> wrote:
Could you explain that further? Are you saying you need an override check for dynamically created/modified parent classes such that static analysis would not suffice? Your second point about people submitting PRs whereby they add decorators
You're right (and so is Nick): the static analysis would suffice. I guess you don't get the automatic docstring copying. My personal feeling is that it is also nice to have (even if I don't use it) support for dynamically created/modified parent classes. Best, Neil On Tue, Jul 19, 2016 at 11:32 AM Michael Selik <michael.selik@gmail.com> wrote:
On Jul 19, 2016 10:43 AM, "Nick Coghlan" <ncoghlan@gmail.com> wrote:
Is there enough information, in all cases, to do the check at definition time? - Obviously this wouldnt work for AOP-style dynamic mixins which are/could be modified by changing {__bases__, __class__, ...?} at runtime (certainly an infrequent use case and a different style of composition than @override-relevant inheritance) - __new__
What reasons are there for @override to be in stdlib?: - no import required (though it would still be necessary for backwards compatibility) - cross-cutting concerns - definition-time type verificayions - runtime type verifications aside: Spring Python may be of interest to people more familiar with Java
On Tue, Jul 19, 2016, 12:08 PM Wes Turner <wes.turner@gmail.com> wrote:
This is an argument for adding this feature to the typing module as opposed to a decorator, correct? Changing the base dynamically would not trigger another call to @overrides, even with some fancy metaclassing. The check wouldn't occur until instantiation or calling the method. Going for instantiation-time checks could get awkward via metaclass. To avoid conflicts I guess OverrideMeta would be an extension of ABCMeta. If just using __new__ it's too easy to accidentally bypass. Call-time checks would add overhead to every method call.
On Tue, Jul 19, 2016 at 03:18:34PM +1000, Nick Coghlan wrote:
On 19 July 2016 at 14:04, Guido van Rossum <guido@python.org> wrote in response to my comments:
The same argument would apply against abstractmethod...
Well, to be honest, I've never been in a situation where I either needed abstractmethod or knowingly had to deal with a class that used it. Maybe I just don't use classes enough to see why this proposal is useful.
Isn't that one of the features of IDEs? (Which doesn't help those who aren't using an IDE.)
But requiring people to always decorate overrides seems hard, unless we made it a property that can be enforced by a metaclass, maybe?
[Nick]
Indeed.
Certainly this is something which static checkers *could* do, but surely a runtime check would be sufficient? The runtime check only needs to occur when the class is defined, not on every call. It could look something like this: def override(method): proxy = get_class_proxy() # MAGIC GOES HERE assert hasattr(method.__name__, proxy) return method I say MAGIC GOES HERE because of course the class doesn't exist yet. The get_class_proxy() function should return some sort of proxy to the soon-to-exist class' MRO. (Maybe this is harder to write than I thought.) Having it work at class definition time rather than compile-time means that it will work with odd corner cases like injected methods: class C: pass ... ... C.method = something() class D(C): @override def method(self): pass I wouldn't expect a purely static checker to necessarily realise that C.method exists, but there's no problem for the runtime check.
-- Steve
On Tue, Jul 19, 2016 at 7:51 AM Steven D'Aprano <steve@pearwood.info> wrote:
(Maybe this is harder to write than I thought.)
I don't think you need any magic. You can either do it like @abstractmethod whereby you put the logic in ABCMeta or given that PEP 487 has been accepted, put the logic in a plain old base class (ABCBase?) that has the logic in __init_subclass__. This would work with your corner case. It also is a bit more flexible. Having it work at class definition time rather than compile-time means
participants (10)
-
Ben Finney -
Ethan Furman -
Guido van Rossum -
Michael Selik -
Neil Girdhar -
Nick Coghlan -
Shrey Desai -
Steven D'Aprano -
Terry Reedy -
Wes Turner