This is part five (c), and focuses on sharing strict types between
parser.py and expr.py.

gitlab: https://gitlab.com/jsnow/qemu/-/commits/python-qapi-cleanup-pt5c

Every commit should pass with:
 - `isort -c qapi/`
 - `flake8 qapi/`
 - `pylint --rcfile=qapi/pylintrc qapi/`
 - `mypy --config-file=qapi/mypy.ini qapi/`

V4:
 - Dropped the "split check_exprs" patch
 - Based the QAPIExpression class on Dict[str, object] instead
 - Removed the type workaround patch, which is no longer needed.
 - Added a new patch to fix a problem with Python 3.6 and pylint

V3:
 - Squashed a bunch of patches into the QAPIExpression patch
 - Added a few 'info' locals whenever there were >= 3 usages to help
   minimize some churn
 - Removed some type redundancy from docstrings
 - Brought along the two patches from pt0 that I want merged.
 - Removed 'pexpr' entirely, there's no intermediate state where it's
   needed now.
 - Minor style issues.

John Snow (6):
  qapi: Update flake8 config
  qapi: update pylint configuration
  qapi: Add minor typing workaround for 3.6
  qapi/parser: add QAPIExpression type
  qapi: remove _JSONObject
  qapi: remove JSON value FIXME

 scripts/qapi/.flake8   |   3 +-
 scripts/qapi/expr.py   | 101 ++++++++++++++++-------------------------
 scripts/qapi/parser.py |  41 +++++++++--------
 scripts/qapi/pylintrc  |   1 +
 scripts/qapi/schema.py |  72 +++++++++++++++--------------
 5 files changed, 104 insertions(+), 114 deletions(-)

-- 
2.39.0

I had a few suggestions, but none of them requires a respin.  Let's
discuss them, and then I merge.

On Wed, Feb 15, 2023 at 8:39 AM Markus Armbruster <armbru@redhat.com> wrote:
>
> I had a few suggestions, but none of them requires a respin.  Let's
> discuss them, and then I merge.

Hiya, I lost track of things a little due to the other Python
discussion. Who is waiting for whom?

--js

John Snow <jsnow@redhat.com> writes:

> On Wed, Feb 15, 2023 at 8:39 AM Markus Armbruster <armbru@redhat.com> wrote:
>>
>> I had a few suggestions, but none of them requires a respin.  Let's
>> discuss them, and then I merge.
>
> Hiya, I lost track of things a little due to the other Python
> discussion. Who is waiting for whom?

Just two questions remain:

* PATCH 3: Dumb down check_keys() argument all the way to List[str]?

* PATCH 4: Suggested commit message addition okay?

We settle them, and then I'll take it from there.

On Tue, Feb 21, 2023, 1:42 AM Markus Armbruster <armbru@redhat.com> wrote:

> John Snow <jsnow@redhat.com> writes:
>
> > On Wed, Feb 15, 2023 at 8:39 AM Markus Armbruster <armbru@redhat.com>
> wrote:
> >>
> >> I had a few suggestions, but none of them requires a respin.  Let's
> >> discuss them, and then I merge.
> >
> > Hiya, I lost track of things a little due to the other Python
> > discussion. Who is waiting for whom?
>
> Just two questions remain:
>
> * PATCH 3: Dumb down check_keys() argument all the way to List[str]?
>

Kinda prefer not to, but maybe I'm being too precious. (I have some more
exploratory patches that do use tuples here instead, but admit it's not
crucial.)

From a pure typing perspective, I wish I could leave it as it is now,
because I prefer to type input types as loosely as possible: claim only the
minimum power we need, instead of enforcing the specificity we happen to
have.

With the bug for 3.6 that is forcing me to use a more specific type anyway,
maybe you're right and I should just use List[] until I'm allowed to have
my proper abstraction.

OK, before I go further, lemme say that you can change it to List[] if you
want. I won't be too precious about it. (You can rewrite the patch in
question if you don't want to wait 24h.)

But, a question about typing strategy:

As a python tooling maintainer, should I push people to type as flexible as
possible or as *specific* as possible in general?

Flexible: (e.g. Sequence or Iterable)
- More likely to get along with other code
- More "pythonic", abstractly
- Less useful as documentation; if a function always happens to get a list,
is it annoying to pretend it's merely a sequence?

Specific: (e.g. List)
- Most useful as documentation
- Can assert greater knowledge of all callers
- More power afforded to function ("room to grow"?)
- More likely to require non-local edits when changing functionality or
refactoring
- More likely to require "casts" at callsites to convert data types

I think I lean towards the flexible/broad typing strategy in general, but
lament it cannot be applied appropriately here today.


> * PATCH 4: Suggested commit message addition okay?
>

Yes, ACK.


> We settle them, and then I'll take it from there.
>
>

John Snow <jsnow@redhat.com> writes:

> On Tue, Feb 21, 2023, 1:42 AM Markus Armbruster <armbru@redhat.com> wrote:
>
>> John Snow <jsnow@redhat.com> writes:
>>
>> > On Wed, Feb 15, 2023 at 8:39 AM Markus Armbruster <armbru@redhat.com>
>> wrote:
>> >>
>> >> I had a few suggestions, but none of them requires a respin.  Let's
>> >> discuss them, and then I merge.
>> >
>> > Hiya, I lost track of things a little due to the other Python
>> > discussion. Who is waiting for whom?
>>
>> Just two questions remain:
>>
>> * PATCH 3: Dumb down check_keys() argument all the way to List[str]?
>>
>
> Kinda prefer not to, but maybe I'm being too precious. (I have some more
> exploratory patches that do use tuples here instead, but admit it's not
> crucial.)
>
> From a pure typing perspective, I wish I could leave it as it is now,
> because I prefer to type input types as loosely as possible: claim only the
> minimum power we need, instead of enforcing the specificity we happen to
> have.
>
> With the bug for 3.6 that is forcing me to use a more specific type anyway,
> maybe you're right and I should just use List[] until I'm allowed to have
> my proper abstraction.
>
> OK, before I go further, lemme say that you can change it to List[] if you
> want. I won't be too precious about it. (You can rewrite the patch in
> question if you don't want to wait 24h.)
>
> But, a question about typing strategy:

A good one!

> As a python tooling maintainer, should I push people to type as flexible as
> possible or as *specific* as possible in general?
>
> Flexible: (e.g. Sequence or Iterable)
> - More likely to get along with other code
> - More "pythonic", abstractly
> - Less useful as documentation; if a function always happens to get a list,
> is it annoying to pretend it's merely a sequence?
>
> Specific: (e.g. List)
> - Most useful as documentation
> - Can assert greater knowledge of all callers
> - More power afforded to function ("room to grow"?)
> - More likely to require non-local edits when changing functionality or
> refactoring
> - More likely to require "casts" at callsites to convert data types
>
> I think I lean towards the flexible/broad typing strategy in general, but
> lament it cannot be applied appropriately here today.

Adjusting strategy to context can make some sense, I think.

When we're exposing a library to unknown users, we want to ask for the
weakest possible preconditions, i.e. use maximally abstract types for
input.

That's one end of a spectrum, I think.  Here's an example for the other
end: say I write a helper function for just one caller, which passes
arguments of just one type.  I'd use exactly that type.  KISS.

Much of the time, we're somewhere in between, i.e. in the realm of
judgement calls and taste.

Sticking to weakest possible preconditions ("flexible") everywhere saves
you the trouble of making the judgement calls.  Defensible point of
view.  Not mine, though.

The more abstract the type is, the more flexibility we shift from callee
to callers.  I want to be able to put the flexibility where I expect it
to be needed more.

Moreover, I really, really like types as documentation.  When I see a
fully concrete type List[str], I know all there is to know about the
argument's type.  When I see Collection[str], I know less.  When I need
to know more, I get to chase callers, just as if there was no type at
all.

And most of all, I habitually eschew abstraction unless I see concrete
benefits.  Don't get me wrong, abstraction is awesome!  Probably the
most essential tool in the toolbox.  But like any powerful tool, it
should be used with care.

Back to the QAPI code generator.  So far, there are no unknown users,
simply because all users are right in the tree.  However, the thing has
become large enough to be difficult to keep in your brain all at once.
We don't *want* to know all users anymore.  We want some abstraction.

But where do we want it, and how much?  Where to put abstract
interfaces?

Not within .py files, I'd say.  Between them.  Or maybe just some of
them.  Ah, those pesky judgement calls!

Do I make sense?  Comments?  Objections?

>> * PATCH 4: Suggested commit message addition okay?
>>
>
> Yes, ACK.
>
>
>> We settle them, and then I'll take it from there.

No need for a respin.