From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.0 (2014-02-07) on aws-us-west-2-korg-lkml-1.web.codeaurora.org Received: from lists.gnu.org (lists.gnu.org [209.51.188.17]) (using TLSv1.2 with cipher ECDHE-RSA-AES256-GCM-SHA384 (256/256 bits)) (No client certificate requested) by smtp.lore.kernel.org (Postfix) with ESMTPS id 9CF02C61D96 for ; Tue, 21 Nov 2023 16:29:40 +0000 (UTC) Received: from localhost ([::1] helo=lists1p.gnu.org) by lists.gnu.org with esmtp (Exim 4.90_1) (envelope-from ) id 1r5TcG-00056b-QB; Tue, 21 Nov 2023 11:28:48 -0500 Received: from eggs.gnu.org ([2001:470:142:3::10]) by lists.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r5TcE-000562-0D for qemu-devel@nongnu.org; Tue, 21 Nov 2023 11:28:46 -0500 Received: from us-smtp-delivery-124.mimecast.com ([170.10.129.124]) by eggs.gnu.org with esmtps (TLS1.2:ECDHE_RSA_AES_256_GCM_SHA384:256) (Exim 4.90_1) (envelope-from ) id 1r5Tc3-0001xX-98 for qemu-devel@nongnu.org; Tue, 21 Nov 2023 11:28:45 -0500 DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=redhat.com; s=mimecast20190719; t=1700584114; h=from:from:reply-to:subject:subject:date:date:message-id:message-id: to:to:cc:cc:mime-version:mime-version:content-type:content-type: in-reply-to:in-reply-to:references:references; bh=IAM3vTV3C+kSrl5CA8Q7g3veLiDe+ByrDnpUe0UROaY=; b=CjaiTIhXqgJ134hlWcCZrUdY5pNhwMM7sxaTS1tdxhh+aMXAIbYYH53EZmL0R11PeKcjNC z3OnsBrV44jJZ4GvstM1mGRii2sfyEBHATPM6GMzexU+wl0e9SmqVFrmXRVIjAOlzLOvYI wUDt7t4ko2jm7tRGtHFjFJXVIke9kVo= Received: from mail-pj1-f69.google.com (mail-pj1-f69.google.com [209.85.216.69]) by relay.mimecast.com with ESMTP with STARTTLS (version=TLSv1.3, cipher=TLS_AES_256_GCM_SHA384) id us-mta-523-olEWaISjP1y7tt3VZX4DeQ-1; Tue, 21 Nov 2023 11:28:32 -0500 X-MC-Unique: olEWaISjP1y7tt3VZX4DeQ-1 Received: by mail-pj1-f69.google.com with SMTP id 98e67ed59e1d1-28004d4462dso8341905a91.0 for ; Tue, 21 Nov 2023 08:28:29 -0800 (PST) X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20230601; t=1700584109; x=1701188909; h=cc:to:subject:message-id:date:from:in-reply-to:references :mime-version:x-gm-message-state:from:to:cc:subject:date:message-id :reply-to; bh=IAM3vTV3C+kSrl5CA8Q7g3veLiDe+ByrDnpUe0UROaY=; b=Z4PvS4K6usVr5a+OiTEWNk+e+tHZioIoFZwtE3LhTO4ir9sLy/FdlrMlk28Eyb+bgx 9lwMuvaiEkKXrsBIprD2uYO49eCsDjUPRpX4uyB+sYMisXCgctJifL/hNDjeOz5YlaUI N+JH1qaSElJBSCROmu/5Pd2L3LfVCTvC49EWQTA3z0/kDBKoLuzpRf1e+YEhHF1pablo oMy3DlR2fm7fqjiUCcQ2NjkGUINs5MS2e8Km/fE58E8rBjyhXvGXfgqdkNgWVNHH5jPJ N50JcNXYXkXgprufb00bzeWLcLYBdaBH+UwN/lr90Qb9+0Zbikx9REkwBz7FjmJuWBVN qMZw== X-Gm-Message-State: AOJu0YwNuUOEcq/uXWzWBwPzBr+yEGg4P2Pwbv5HF/lknO7aCp7xEmIK jwStVedQOxS0zzWp3Ln1S7T6yIT7aHpZeqqwmTHgx6384/lEKJemiuiW9c5gdOLMbQzyFJJne6Q Se8IKsEHTJLWuPBCE4ixRqOLivXnbvns= X-Received: by 2002:a17:90b:17c6:b0:280:cc88:2a46 with SMTP id me6-20020a17090b17c600b00280cc882a46mr11409056pjb.4.1700584108858; Tue, 21 Nov 2023 08:28:28 -0800 (PST) X-Google-Smtp-Source: AGHT+IECmo5jDpUJgE3OfXcxDI/79iVCaIdMA12VbQHlkThQIkmCyp54TdR4aIUV6voI6QBqtj/AfG4+8RsnyNOHfcM= X-Received: by 2002:a17:90b:17c6:b0:280:cc88:2a46 with SMTP id me6-20020a17090b17c600b00280cc882a46mr11409039pjb.4.1700584108503; Tue, 21 Nov 2023 08:28:28 -0800 (PST) MIME-Version: 1.0 References: <20231116014350.653792-1-jsnow@redhat.com> <20231116014350.653792-6-jsnow@redhat.com> <87jzqb4495.fsf@pond.sub.org> In-Reply-To: From: John Snow Date: Tue, 21 Nov 2023 11:28:17 -0500 Message-ID: Subject: Re: [PATCH 05/19] qapi/schema: make c_type() and json_type() abstract methods To: =?UTF-8?Q?Daniel_P=2E_Berrang=C3=A9?= Cc: Markus Armbruster , qemu-devel , Peter Maydell , Michael Roth Content-Type: multipart/alternative; boundary="000000000000b2d283060aac1758" Received-SPF: pass client-ip=170.10.129.124; envelope-from=jsnow@redhat.com; helo=us-smtp-delivery-124.mimecast.com X-Spam_score_int: -20 X-Spam_score: -2.1 X-Spam_bar: -- X-Spam_report: (-2.1 / 5.0 requ) BAYES_00=-1.9, DKIMWL_WL_HIGH=-0.001, DKIM_SIGNED=0.1, DKIM_VALID=-0.1, DKIM_VALID_AU=-0.1, DKIM_VALID_EF=-0.1, HTML_MESSAGE=0.001, SPF_HELO_NONE=0.001, SPF_PASS=-0.001, T_SCC_BODY_TEXT_LINE=-0.01 autolearn=ham autolearn_force=no X-Spam_action: no action X-BeenThere: qemu-devel@nongnu.org X-Mailman-Version: 2.1.29 Precedence: list List-Id: List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org Sender: qemu-devel-bounces+qemu-devel=archiver.kernel.org@nongnu.org --000000000000b2d283060aac1758 Content-Type: text/plain; charset="UTF-8" Content-Transfer-Encoding: quoted-printable On Tue, Nov 21, 2023, 8:43 AM Daniel P. Berrang=C3=A9 wrote: > On Tue, Nov 21, 2023 at 02:36:54PM +0100, Markus Armbruster wrote: > > John Snow 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 meth= od > > > with the proper return type. > > > > > > Signed-off-by: John Snow > > > --- > > > 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 =3D { > > > > 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=3DABCMeta): > @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 > > With regards, > Daniel > -- > |: https://berrange.com -o- > https://www.flickr.com/photos/dberrange :| > |: https://libvirt.org -o- > https://fstop138.berrange.com :| > |: https://entangle-photo.org -o- > https://www.instagram.com/dberrange :| > 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. --js > --000000000000b2d283060aac1758 Content-Type: text/html; charset="UTF-8" Content-Transfer-Encoding: quoted-printable


On Tue, Nov 21, 2023, 8:43 AM Daniel P. Berrang=C3=A9 = <berrange@redhat.com> wrot= e:
On Tue, Nov 21, 2023 at 02:36:54= PM +0100, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
>
> > These methods should always return a str, it's only the defau= lt abstract
> > implementation that doesn't. They can be marked "abstrac= t" by raising
> > NotImplementedError(), which requires subclasses to override the = method
> > with the proper return type.
> >
> > Signed-off-by: John Snow <jsnow@redhat.com>
> > ---
> >=C2=A0 scripts/qapi/schema.py | 8 ++++----
> >=C2=A0 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):
> >=C2=A0 class QAPISchemaType(QAPISchemaEntity):
> >=C2=A0 =C2=A0 =C2=A0 # Return the C type for common use.
> >=C2=A0 =C2=A0 =C2=A0 # For the types we commonly box, this is a po= inter type.
> > -=C2=A0 =C2=A0 def c_type(self):
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 pass
> > +=C2=A0 =C2=A0 def c_type(self) -> str:
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 raise NotImplementedError()
> >=C2=A0
> >=C2=A0 =C2=A0 =C2=A0 # Return the C type to be used in a parameter= list.
> >=C2=A0 =C2=A0 =C2=A0 def c_param_type(self):
> > @@ -244,8 +244,8 @@ def c_param_type(self):
> >=C2=A0 =C2=A0 =C2=A0 def c_unboxed_type(self):
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 return self.c_type()
> >=C2=A0
> > -=C2=A0 =C2=A0 def json_type(self):
> > -=C2=A0 =C2=A0 =C2=A0 =C2=A0 pass
> > +=C2=A0 =C2=A0 def json_type(self) -> str:
> > +=C2=A0 =C2=A0 =C2=A0 =C2=A0 raise NotImplementedError()
> >=C2=A0
> >=C2=A0 =C2=A0 =C2=A0 def alternate_qtype(self):
> >=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 json2qtype =3D {
>
> I wish abstract methods could be done in a more concise way.

The canonical way would be to use the 'abc' module:

=C2=A0 from abc import ABCMeta, abstractmethod

=C2=A0 class A(metaclass=3DABCMeta):
=C2=A0 =C2=A0 =C2=A0 @abstractmethod
=C2=A0 =C2=A0 =C2=A0 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 bo= dy
impl

With regards,
Daniel
--
|: https://berrange.com=C2=A0 =C2=A0 =C2=A0 -o-=C2=A0 =C2=A0 https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org=C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0-o-=C2=A0 = =C2=A0 =C2=A0 =C2=A0 =C2=A0 =C2=A0 https://fstop138.berrange= .com :|
|: https://entangle-photo.org=C2=A0 =C2=A0 -o-=C2=A0 =C2=A0= https://www.instagram.com/dberrange :|

In practice= , I'm under the belief that mypy and pylint both recognize a method rai= sing NotImplementedError as marking an abstract method without needing to e= xplicitly inherit from the ABC.

I suppose there's also=C2=A0https://docs.python.org/3/library/= abc.html#abc.abstractmethod which I am guessing just does this same thi= ng. 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.

--js=
--000000000000b2d283060aac1758--