[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [PATCH v2 31/38] qapi/introspect.py: add _gen_features helper
From: |
Eduardo Habkost |
Subject: |
Re: [PATCH v2 31/38] qapi/introspect.py: add _gen_features helper |
Date: |
Wed, 23 Sep 2020 17:54:38 -0400 |
On Wed, Sep 23, 2020 at 05:43:45PM -0400, John Snow wrote:
> On 9/23/20 12:35 PM, Eduardo Habkost wrote:
> > I believe these two lines above should be removed, as suggested
> > in patch 30, but let's ignore that for now.
> >
>
> Yup, headed there.
>
> > > - if features:
> > > - obj['features'] = [(f.name, Extra(None, f.ifcond)) for f in
> > > features]
> > I can't say I understand completely why moving these two lines
> > outside _make_tree() is useful, but if it makes the cleanup work
> > you did easier, I trust this is the right thing to do. The
> > changes look correct.
>
> The basic premise is:
>
> Why pass information you want to add to obj['features'] to a function to
> make that assignment, when you could just perform that assignment yourself?
>
> Otherwise, _make_tree, which accepts any arbitrary object (not just dicts!)
> has to interrogate its arguments to make sure you gave it a dict when you
> give it a features argument.
Oh, I was not aware that obj could be not a dictionary. In this
case, it makes lots of sense to move the magic outside the
function.
>
> Type-wise, it's cleaner to perform this transformation when we KNOW we have
> an object than it is to defer to a more abstracted function and
> assert/downcast back to more specific types.
--
Eduardo
[PATCH v2 33/38] qapi/introspect.py: add type hint annotations, John Snow, 2020/09/22
[PATCH v2 32/38] qapi/introspect.py: create a typed 'Node' data structure, John Snow, 2020/09/22