Daniel P. Berrangé <berra...@redhat.com> writes:
> On Tue, Nov 21, 2023 at 11:28:17AM -0500, John Snow wrote:
>> On Tue, Nov 21, 2023, 8:43 AM Daniel P. Berrangé <berra...@redhat.com>
>> wrote:
>>
>> > On Tue, Nov 21, 2023 at 02:36:54PM +0100, Markus Armbruster wrote:
>> > > John Snow <js...@redhat.com> writes:
>> > >
>> > > > These methods should always return a str, it's only the default
>> > abstract
>> > > > implementation that doesn't. They can be marked "abstract" by raising
>> > > > NotImplementedError(), which requires subclasses to override the method
>> > > > with the proper return type.
>> > > >
>> > > > Signed-off-by: John Snow <js...@redhat.com>
>> > > > ---
>> > > > scripts/qapi/schema.py | 8 ++++----
>> > > > 1 file changed, 4 insertions(+), 4 deletions(-)
>> > > >
>> > > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py
>> > > > index c5fdd625452..4600a566005 100644
>> > > > --- a/scripts/qapi/schema.py
>> > > > +++ b/scripts/qapi/schema.py
>> > > > @@ -233,8 +233,8 @@ def visit(self, visitor):
>> > > > class QAPISchemaType(QAPISchemaEntity):
>> > > > # Return the C type for common use.
>> > > > # For the types we commonly box, this is a pointer type.
>> > > > - def c_type(self):
>> > > > - pass
>> > > > + def c_type(self) -> str:
>> > > > + raise NotImplementedError()
>> > > >
>> > > > # Return the C type to be used in a parameter list.
>> > > > def c_param_type(self):
>> > > > @@ -244,8 +244,8 @@ def c_param_type(self):
>> > > > def c_unboxed_type(self):
>> > > > return self.c_type()
>> > > >
>> > > > - def json_type(self):
>> > > > - pass
>> > > > + def json_type(self) -> str:
>> > > > + raise NotImplementedError()
>> > > >
>> > > > def alternate_qtype(self):
>> > > > json2qtype = {
>> > >
>> > > I wish abstract methods could be done in a more concise way.
>> >
>> > The canonical way would be to use the 'abc' module:
>> >
>> > from abc import ABCMeta, abstractmethod
>> >
>> > class A(metaclass=ABCMeta):
>> > @abstractmethod
>> > def foo(self): pass
>> >
>> > Not sure if the @abstractmethod decorator is enough to keep the static
>> > typing checker happy about a 'str' return type, when there is no body
>> > impl
>>
>> In practice, I'm under the belief that mypy and pylint both recognize a
>> method raising NotImplementedError as marking an abstract method without
>> needing to explicitly inherit from the ABC.
>>
>> I suppose there's also
>> https://docs.python.org/3/library/abc.html#abc.abstractmethod which I am
>> guessing just does this same thing. I'll see what makes mypy happy. I'm
>> assuming Markus would like to see something like this decorator to make it
>> more obvious that it's an abstract method.
>
> The 'abc' module described is an official PEP standard
>
> https://peps.python.org/pep-3119/
Compare:
@abstractmethod
def c_type(self) -> str:
pass
def c_type(self) -> str:
raise NotImplementedError()
I prefer the former, because it's more explicit.
Bonus: prevents accidental instantiation, and sub-classes don't need to
know what's abstract in the super-class, they can blindly use super()
calls. docs.python.org:
Using this decorator requires that the class’s metaclass is ABCMeta
or is derived from it. A class that has a metaclass derived from
ABCMeta cannot be instantiated unless all of its abstract methods
and properties are overridden. The abstract methods can be called
using any of the normal ‘super’ call mechanisms. abstractmethod()
may be used to declare abstract methods for properties and
descriptors.
Hardly matters here, but since it's free...
