| Message ID | 20231116014350.653792-11-jsnow@redhat.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | qapi: statically type schema.py | expand |
John Snow <jsnow@redhat.com> writes: > This field should always be present and defined. Change this to be a > runtime @property that can emit an error if it's called prior to > check(). > > This helps simplify typing by avoiding the need to interrogate the value > for None at multiple callsites. > > RFC: Yes, this is a slightly different technique than the one I used for > QAPISchemaObjectTypeMember.type; In PATCH 04. > I think I prefer this one as being a > little less hokey, but it is more SLOC. Dealer's choice for which style > wins out -- now you have an example of both. Thanks for letting us see both. I believe all the extra lines accomplish is a different exception RuntimeError with a custom message vs. plain AttributeError. I don't think the more elaborate exception is worth the extra code. > Signed-off-by: John Snow <jsnow@redhat.com> > --- > scripts/qapi/schema.py | 13 +++++++++++-- > 1 file changed, 11 insertions(+), 2 deletions(-) > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > index c9a194103e1..462acb2bb61 100644 > --- a/scripts/qapi/schema.py > +++ b/scripts/qapi/schema.py > @@ -366,7 +366,16 @@ def __init__(self, name, info, element_type): > super().__init__(name, info, None) > assert isinstance(element_type, str) > self._element_type_name = element_type > - self.element_type = None > + self._element_type: Optional[QAPISchemaType] = None > + > + @property > + def element_type(self) -> QAPISchemaType: > + if self._element_type is None: > + raise RuntimeError( > + "QAPISchemaArray has no element_type until " > + "after check() has been run." > + ) > + return self._element_type > > def need_has_if_optional(self): > # When FOO is an array, we still need has_FOO to distinguish > @@ -375,7 +384,7 @@ def need_has_if_optional(self): > > def check(self, schema): > super().check(schema) > - self.element_type = schema.resolve_type( > + self._element_type = schema.resolve_type( > self._element_type_name, self.info, > self.info and self.info.defn_meta) > assert not isinstance(self.element_type, QAPISchemaArrayType)
On Tue, Nov 21, 2023, 9:28 AM Markus Armbruster <armbru@redhat.com> wrote: > John Snow <jsnow@redhat.com> writes: > > > This field should always be present and defined. Change this to be a > > runtime @property that can emit an error if it's called prior to > > check(). > > > > This helps simplify typing by avoiding the need to interrogate the value > > for None at multiple callsites. > > > > RFC: Yes, this is a slightly different technique than the one I used for > > QAPISchemaObjectTypeMember.type; > > In PATCH 04. > > > I think I prefer this one as being a > > little less hokey, but it is more SLOC. Dealer's choice for which style > > wins out -- now you have an example of both. > > Thanks for letting us see both. > My pleasure ;) > I believe all the extra lines accomplish is a different exception > RuntimeError with a custom message vs. plain AttributeError. > > I don't think the more elaborate exception is worth the extra code. > Hmm, shame. You're the boss :) > > Signed-off-by: John Snow <jsnow@redhat.com> > > --- > > scripts/qapi/schema.py | 13 +++++++++++-- > > 1 file changed, 11 insertions(+), 2 deletions(-) > > > > diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py > > index c9a194103e1..462acb2bb61 100644 > > --- a/scripts/qapi/schema.py > > +++ b/scripts/qapi/schema.py > > @@ -366,7 +366,16 @@ def __init__(self, name, info, element_type): > > super().__init__(name, info, None) > > assert isinstance(element_type, str) > > self._element_type_name = element_type > > - self.element_type = None > > + self._element_type: Optional[QAPISchemaType] = None > > + > > + @property > > + def element_type(self) -> QAPISchemaType: > > + if self._element_type is None: > > + raise RuntimeError( > > + "QAPISchemaArray has no element_type until " > > + "after check() has been run." > > + ) > > + return self._element_type > > > > def need_has_if_optional(self): > > # When FOO is an array, we still need has_FOO to distinguish > > @@ -375,7 +384,7 @@ def need_has_if_optional(self): > > > > def check(self, schema): > > super().check(schema) > > - self.element_type = schema.resolve_type( > > + self._element_type = schema.resolve_type( > > self._element_type_name, self.info, > > self.info and self.info.defn_meta) > > assert not isinstance(self.element_type, QAPISchemaArrayType) > >
diff --git a/scripts/qapi/schema.py b/scripts/qapi/schema.py index c9a194103e1..462acb2bb61 100644 --- a/scripts/qapi/schema.py +++ b/scripts/qapi/schema.py @@ -366,7 +366,16 @@ def __init__(self, name, info, element_type): super().__init__(name, info, None) assert isinstance(element_type, str) self._element_type_name = element_type - self.element_type = None + self._element_type: Optional[QAPISchemaType] = None + + @property + def element_type(self) -> QAPISchemaType: + if self._element_type is None: + raise RuntimeError( + "QAPISchemaArray has no element_type until " + "after check() has been run." + ) + return self._element_type def need_has_if_optional(self): # When FOO is an array, we still need has_FOO to distinguish @@ -375,7 +384,7 @@ def need_has_if_optional(self): def check(self, schema): super().check(schema) - self.element_type = schema.resolve_type( + self._element_type = schema.resolve_type( self._element_type_name, self.info, self.info and self.info.defn_meta) assert not isinstance(self.element_type, QAPISchemaArrayType)
This field should always be present and defined. Change this to be a runtime @property that can emit an error if it's called prior to check(). This helps simplify typing by avoiding the need to interrogate the value for None at multiple callsites. RFC: Yes, this is a slightly different technique than the one I used for QAPISchemaObjectTypeMember.type; I think I prefer this one as being a little less hokey, but it is more SLOC. Dealer's choice for which style wins out -- now you have an example of both. Signed-off-by: John Snow <jsnow@redhat.com> --- scripts/qapi/schema.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-)