Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
On 4/21/21 9:58 AM, Markus Armbruster wrote: John Snow writes: [...] I've made a re-spin. Let's try something new, if you don't mind: I've pushed a "almost v5" copy onto my gitlab, where edits made against this patch are in their own commit so that all of the pending edits I've made are easily visible. Here's the "merge request", which I made against my own fork of master: https://gitlab.com/jsnow/qemu/-/merge_requests/1/diffs (It's marked "WIP", so there's no risk of me accidentally merging it -- and if I did, it would be to my own "master" branch, so no worries about us goofing this up.) If you click "Commits (21)" at the top, underneath "WIP: python-qapi-cleanup-pt3", you can see the list of commits in the re-spin. (Four of these commits are the DO-NOT-MERGE ones I carry around as a testing pre-requisite.) From here, you can see the "[RFC] docstring diff" patch which shows all the edits I've made so far based on your feedback and my tinkering. https://gitlab.com/jsnow/qemu/-/merge_requests/1/diffs?commit_id=3f0e9fb71304edb381ce3b9bf0ff08624fb277bc I invite you to leave feedback here on this view (and anywhere else in the series that still needs adjusting, if you are so willing to humor me) by highlighting the line and clicking the comment box icon on the left. If you left-click and drag the comment box, you can target a range of lines. (You can even propose a diff directly using this method, which allows me to just accept your proposal directly.) If you leave any comments here, I can resolve each individual nugget of feedback by clicking "Resolve Thread" in my view, which will help me keep track of which items I believe I have addressed and which items I have not. This will help me make sure I don't miss any of your feedback, and it helps me keep track of what edits I've made for the next changelog. Willing to try it out? Once we're both happy with it, I will send it back to the list for final assessment using our traditional process. Anyone else who wants to come comment on the gitlab draft is of course more than welcome to. I have only a few minor remarks, and I'm too lazy to create a gitlab account just for them. (You'll need one eventually, I think. There was talk of requiring maintainers all to have one in order to run CI by submitting a MR on the main repo as an alternative PR workflow up to Peter Maydell to reduce CI hours. Humor me and make one? I really would like to try it out. Maybe for part 5? I want to see if it helps me be more organized when making a large number of edits, and I think it might help me implement more of your suggestions. At least, that's how I'm selling it!) * Commit 3f0e9fb713 qapi/expr: [RFC] docstring diff - You mixed up check_name_lower() and check_name_camel() @_@ oops. Too many nearly identical code regions. Thanks. - Nitpick: check_defn_name_str() has inconsistent function name markup. Good spot. It doesn't matter to sphinx, but you expressed a preference for seeing the empty parens to help intuit the type of symbol being referenced when reading the plaintext and I agree. - I'd like to suggest a tweak of check_defn_name_str() :param meta: Sounds good, thanks! Anything that makes "type" less ambiguous is graciously welcome. That's all. Converged quickly. Nice! Incremental diff appended. * Old "[PATCH v4 17/19] qapi/expr.py: Use tuples instead of lists for static data" is gone. I think this leaves commit 913e3fd6f8's "Later patches will make use of that" dangling. Let's not drop old PATCH 17. Put it right after 913e3fd6f8 if that's trivial. If not, put it wherever it creates the least work for you. OK, I can un-plunk it. diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index f2bb92ab79..5c9060cb1b 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -124,7 +124,7 @@ def check_name_lower(name: str, info: QAPISourceInfo, source: str, permit_upper: bool = False, permit_underscore: bool = False) -> None: """ -Ensure that ``name`` is a valid user defined type name. +Ensure that ``name`` is a valid command or member name. This means it must be a valid QAPI name as checked by `check_name_str()`, but where the stem prohibits uppercase @@ -147,7 +147,7 @@ def check_name_lower(name: str, info: QAPISourceInfo, source: str, def check_name_camel(name: str, info: QAPISourceInfo, source: str) -> None: """ -Ensure that ``name`` is a valid command or member name. +Ensure that ``name`` is a valid user-defined type name. This means it must be a valid QAPI name as checked by `check_name_str()`, but where the stem must be in CamelCase. @@ -168,14 +168,14 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None: Ensure that ``name`` is a valid definition name. Based on the value of ``meta``, this means that: - -
Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
John Snow writes: [...] > I've made a re-spin. Let's try something new, if you don't mind: > > I've pushed a "almost v5" copy onto my gitlab, where edits made against > this patch are in their own commit so that all of the pending edits I've > made are easily visible. > > Here's the "merge request", which I made against my own fork of master: > https://gitlab.com/jsnow/qemu/-/merge_requests/1/diffs > > (It's marked "WIP", so there's no risk of me accidentally merging it -- > and if I did, it would be to my own "master" branch, so no worries about > us goofing this up.) > > If you click "Commits (21)" at the top, underneath "WIP: > python-qapi-cleanup-pt3", you can see the list of commits in the re-spin. > > (Four of these commits are the DO-NOT-MERGE ones I carry around as a > testing pre-requisite.) > > From here, you can see the "[RFC] docstring diff" patch which shows all > the edits I've made so far based on your feedback and my tinkering. > > https://gitlab.com/jsnow/qemu/-/merge_requests/1/diffs?commit_id=3f0e9fb71304edb381ce3b9bf0ff08624fb277bc > > I invite you to leave feedback here on this view (and anywhere else in > the series that still needs adjusting, if you are so willing to humor > me) by highlighting the line and clicking the comment box icon on the > left. If you left-click and drag the comment box, you can target a range > of lines. > > (You can even propose a diff directly using this method, which allows me > to just accept your proposal directly.) > > If you leave any comments here, I can resolve each individual nugget of > feedback by clicking "Resolve Thread" in my view, which will help me > keep track of which items I believe I have addressed and which items I > have not. This will help me make sure I don't miss any of your feedback, > and it helps me keep track of what edits I've made for the next changelog. > > Willing to try it out? > > Once we're both happy with it, I will send it back to the list for final > assessment using our traditional process. Anyone else who wants to come > comment on the gitlab draft is of course more than welcome to. I have only a few minor remarks, and I'm too lazy to create a gitlab account just for them. * Commit 3f0e9fb713 qapi/expr: [RFC] docstring diff - You mixed up check_name_lower() and check_name_camel() - Nitpick: check_defn_name_str() has inconsistent function name markup. - I'd like to suggest a tweak of check_defn_name_str() :param meta: That's all. Converged quickly. Nice! Incremental diff appended. * Old "[PATCH v4 17/19] qapi/expr.py: Use tuples instead of lists for static data" is gone. I think this leaves commit 913e3fd6f8's "Later patches will make use of that" dangling. Let's not drop old PATCH 17. Put it right after 913e3fd6f8 if that's trivial. If not, put it wherever it creates the least work for you. diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py index f2bb92ab79..5c9060cb1b 100644 --- a/scripts/qapi/expr.py +++ b/scripts/qapi/expr.py @@ -124,7 +124,7 @@ def check_name_lower(name: str, info: QAPISourceInfo, source: str, permit_upper: bool = False, permit_underscore: bool = False) -> None: """ -Ensure that ``name`` is a valid user defined type name. +Ensure that ``name`` is a valid command or member name. This means it must be a valid QAPI name as checked by `check_name_str()`, but where the stem prohibits uppercase @@ -147,7 +147,7 @@ def check_name_lower(name: str, info: QAPISourceInfo, source: str, def check_name_camel(name: str, info: QAPISourceInfo, source: str) -> None: """ -Ensure that ``name`` is a valid command or member name. +Ensure that ``name`` is a valid user-defined type name. This means it must be a valid QAPI name as checked by `check_name_str()`, but where the stem must be in CamelCase. @@ -168,14 +168,14 @@ def check_defn_name_str(name: str, info: QAPISourceInfo, meta: str) -> None: Ensure that ``name`` is a valid definition name. Based on the value of ``meta``, this means that: - - 'event' names adhere to `check_name_upper`. - - 'command' names adhere to `check_name_lower`. + - 'event' names adhere to `check_name_upper()`. + - 'command' names adhere to `check_name_lower()`. - Else, meta is a type, and must pass `check_name_camel()`. These names must not end with ``Kind`` nor ``List``. :param name: Name to check. :param info: QAPI schema source file information. -:param meta: Type name of the QAPI expression. +:param meta: Meta-type name of the QAPI expression. :raise QAPISemError: When ``name`` fails validation. """
Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
On 4/17/21 9:18 AM, Markus Armbruster wrote:
John Snow writes:
On 4/14/21 11:04 AM, Markus Armbruster wrote:
John Snow writes:
Thanks for taking this on. I realize it's a slog.
(Update: much later: AUUUGH WHY DID I DECIDE TO WRITE DOCS. MY HUBRIS)
LOL!
Signed-off-by: John Snow
---
scripts/qapi/expr.py | 213 ++-
1 file changed, 208 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 1869ddf815..adc5b903bc 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -1,7 +1,5 @@
# -*- coding: utf-8 -*-
#
-# Check (context-free) QAPI schema expression structure
-#
# Copyright IBM, Corp. 2011
# Copyright (c) 2013-2019 Red Hat Inc.
#
@@ -14,6 +12,25 @@
# This work is licensed under the terms of the GNU GPL, version 2.
# See the COPYING file in the top-level directory.
+"""
+Normalize and validate (context-free) QAPI schema expression structures.
+
+After QAPI expressions are parsed from disk, they are stored in
+recursively nested Python data structures using Dict, List, str, bool,
+and int. This module ensures that those nested structures have the
+correct type(s) and key(s) where appropriate for the QAPI context-free
+grammar.
"from disk"? Perhaps something like "QAPISchemaParser parses the QAPI
schema into abstract syntax trees consisting of dict, list, str, bool
and int nodes." This isn't quite accurate; it parses into a list of
{'expr': AST, 'info': INFO}, but that's detail.
Let's skip the detail; it doesn't help communicate purpose in the first
paragraph here at the high level. The bulk of this module IS primarily
concerned with the structures named.
Edited to:
`QAPISchemaParser` parses a QAPI schema into abstract syntax trees
consisting of dict, list, str, bool, and int nodes. This module ensures
that these nested structures have the correct type(s) and key(s) where
appropriate for the QAPI context-free grammar.
(I replaced "the QAPI schema" with "a QAPI schema" as we have several;
and I wanted to hint at (somehow) that this processes configurable input
(i.e. "from disk") and not something indelibly linked.)
((What's wrong with "from disk?"))
It can come from anywhere:
$ python3 scripts/qapi-gen.py /dev/stdin
{'command': 'testme'}
PEP 8: You should use two spaces after a sentence-ending period in
multi- sentence comments, except after the final sentence.
Is this a demand?
It's a polite request to save me the (minor) trouble to fix it up in my
tree :)
:(
(I took a stab at it. May have missed a spot or two...)
+
+The QAPI schema expression language allows for syntactic sugar; this
suggest "certain syntactic sugar".
OK
+module also handles the normalization process of these nested
+structures.
+
+See `check_exprs` for the main entry point.
+
+See `schema.QAPISchema` for processing into native Python data
+structures and contextual semantic validation.
+"""
+
import re
from typing import (
Collection,
@@ -31,9 +48,10 @@
from .source import QAPISourceInfo
-# Deserialized JSON objects as returned by the parser;
-# The values of this mapping are not necessary to exhaustively type
-# here, because the purpose of this module is to interrogate that type.
+#: Deserialized JSON objects as returned by the parser.
+#:
+#: The values of this mapping are not necessary to exhaustively type
+#: here, because the purpose of this module is to interrogate that type.
First time I see #: comments; pardon my ignorance. What's the deal?
Sphinx-ese: It indicates that this comment is actually a block relating
to the entity below. It also means that I can cross-reference
`_JSONObject` in docstrings.
... which, because of the rewrite where I stopped calling this object an
Expression to avoid overloading a term, is something I actually don't
try to cross-reference anymore.
So this block can be dropped now, actually.
(Also: It came up in part one, actually: I snuck one in for EATSPACE,
and reference it in the comment for cgen. We cannot cross-reference
constants unless they are documented, and this is how we accomplish that.)
I guess it needs to come up a lot more often for me to actually learn
it... Thanks!
I am in love with the idea of a project-wide cross reference system, but
it's been tough to get the right ideas in my head for how to do it.
_JSONObject = Dict[str, object]
@@ -48,11 +66,29 @@
def check_name_is_str(name: object,
info: QAPISourceInfo,
source: str) -> None:
+"""Ensures that ``name`` is a string."""
PEP 257: The docstring [...] prescribes the function or method's effect
as a command ("Do this", "Return that"), not as a description;
e.g. don't write "Returns the pathname ...".
More of the same below.
Alright.
While we're here, then ...
I take this to mean that you prefer:
:raise: to :raises:, and
:return: to :returns: ?
Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
John Snow writes:
> On 4/14/21 11:04 AM, Markus Armbruster wrote:
>> John Snow writes:
>>
>
> Thanks for taking this on. I realize it's a slog.
>
> (Update: much later: AUUUGH WHY DID I DECIDE TO WRITE DOCS. MY HUBRIS)
LOL!
>>> Signed-off-by: John Snow
>>> ---
>>> scripts/qapi/expr.py | 213 ++-
>>> 1 file changed, 208 insertions(+), 5 deletions(-)
>>>
>>> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
>>> index 1869ddf815..adc5b903bc 100644
>>> --- a/scripts/qapi/expr.py
>>> +++ b/scripts/qapi/expr.py
>>> @@ -1,7 +1,5 @@
>>> # -*- coding: utf-8 -*-
>>> #
>>> -# Check (context-free) QAPI schema expression structure
>>> -#
>>> # Copyright IBM, Corp. 2011
>>> # Copyright (c) 2013-2019 Red Hat Inc.
>>> #
>>> @@ -14,6 +12,25 @@
>>> # This work is licensed under the terms of the GNU GPL, version 2.
>>> # See the COPYING file in the top-level directory.
>>>
>>> +"""
>>> +Normalize and validate (context-free) QAPI schema expression structures.
>>> +
>>> +After QAPI expressions are parsed from disk, they are stored in
>>> +recursively nested Python data structures using Dict, List, str, bool,
>>> +and int. This module ensures that those nested structures have the
>>> +correct type(s) and key(s) where appropriate for the QAPI context-free
>>> +grammar.
>>
>> "from disk"? Perhaps something like "QAPISchemaParser parses the QAPI
>> schema into abstract syntax trees consisting of dict, list, str, bool
>> and int nodes." This isn't quite accurate; it parses into a list of
>> {'expr': AST, 'info': INFO}, but that's detail.
>>
>
> Let's skip the detail; it doesn't help communicate purpose in the first
> paragraph here at the high level. The bulk of this module IS primarily
> concerned with the structures named.
>
> Edited to:
>
> `QAPISchemaParser` parses a QAPI schema into abstract syntax trees
> consisting of dict, list, str, bool, and int nodes. This module ensures
> that these nested structures have the correct type(s) and key(s) where
> appropriate for the QAPI context-free grammar.
>
> (I replaced "the QAPI schema" with "a QAPI schema" as we have several;
> and I wanted to hint at (somehow) that this processes configurable input
> (i.e. "from disk") and not something indelibly linked.)
>
> ((What's wrong with "from disk?"))
It can come from anywhere:
$ python3 scripts/qapi-gen.py /dev/stdin
{'command': 'testme'}
>> PEP 8: You should use two spaces after a sentence-ending period in
>> multi- sentence comments, except after the final sentence.
>
> Is this a demand?
It's a polite request to save me the (minor) trouble to fix it up in my
tree :)
>>> +
>>> +The QAPI schema expression language allows for syntactic sugar; this
>>
>> suggest "certain syntactic sugar".
>>
>
> OK
>
>>> +module also handles the normalization process of these nested
>>> +structures.
>>> +
>>> +See `check_exprs` for the main entry point.
>>> +
>>> +See `schema.QAPISchema` for processing into native Python data
>>> +structures and contextual semantic validation.
>>> +"""
>>> +
>>> import re
>>> from typing import (
>>> Collection,
>>> @@ -31,9 +48,10 @@
>>> from .source import QAPISourceInfo
>>>
>>>
>>> -# Deserialized JSON objects as returned by the parser;
>>> -# The values of this mapping are not necessary to exhaustively type
>>> -# here, because the purpose of this module is to interrogate that type.
>>> +#: Deserialized JSON objects as returned by the parser.
>>> +#:
>>> +#: The values of this mapping are not necessary to exhaustively type
>>> +#: here, because the purpose of this module is to interrogate that type.
>>
>> First time I see #: comments; pardon my ignorance. What's the deal?
>>
>
> Sphinx-ese: It indicates that this comment is actually a block relating
> to the entity below. It also means that I can cross-reference
> `_JSONObject` in docstrings.
>
> ... which, because of the rewrite where I stopped calling this object an
> Expression to avoid overloading a term, is something I actually don't
> try to cross-reference anymore.
>
> So this block can be dropped now, actually.
>
> (Also: It came up in part one, actually: I snuck one in for EATSPACE,
> and reference it in the comment for cgen. We cannot cross-reference
> constants unless they are documented, and this is how we accomplish that.)
I guess it needs to come up a lot more often for me to actually learn
it... Thanks!
>>> _JSONObject = Dict[str, object]
>>>
>>>
>>> @@ -48,11 +66,29 @@
>>> def check_name_is_str(name: object,
>>> info: QAPISourceInfo,
>>> source: str) -> None:
>>> +"""Ensures that ``name`` is a string."""
>>
>> PEP 257: The docstring [...] prescribes the function or method's effect
>> as a command ("Do this", "Return that"), not as a description;
>> e.g. don't write "Returns the pathname ...".
>>
>> More of the same below.
>>
>
> Alright.
>
> While we're
Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
On 4/14/21 11:04 AM, Markus Armbruster wrote:
John Snow writes:
Thanks for taking this on. I realize it's a slog.
(Update: much later: AUUUGH WHY DID I DECIDE TO WRITE DOCS. MY HUBRIS)
Signed-off-by: John Snow
---
scripts/qapi/expr.py | 213 ++-
1 file changed, 208 insertions(+), 5 deletions(-)
diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
index 1869ddf815..adc5b903bc 100644
--- a/scripts/qapi/expr.py
+++ b/scripts/qapi/expr.py
@@ -1,7 +1,5 @@
# -*- coding: utf-8 -*-
#
-# Check (context-free) QAPI schema expression structure
-#
# Copyright IBM, Corp. 2011
# Copyright (c) 2013-2019 Red Hat Inc.
#
@@ -14,6 +12,25 @@
# This work is licensed under the terms of the GNU GPL, version 2.
# See the COPYING file in the top-level directory.
+"""
+Normalize and validate (context-free) QAPI schema expression structures.
+
+After QAPI expressions are parsed from disk, they are stored in
+recursively nested Python data structures using Dict, List, str, bool,
+and int. This module ensures that those nested structures have the
+correct type(s) and key(s) where appropriate for the QAPI context-free
+grammar.
"from disk"? Perhaps something like "QAPISchemaParser parses the QAPI
schema into abstract syntax trees consisting of dict, list, str, bool
and int nodes." This isn't quite accurate; it parses into a list of
{'expr': AST, 'info': INFO}, but that's detail.
Let's skip the detail; it doesn't help communicate purpose in the first
paragraph here at the high level. The bulk of this module IS primarily
concerned with the structures named.
Edited to:
`QAPISchemaParser` parses a QAPI schema into abstract syntax trees
consisting of dict, list, str, bool, and int nodes. This module ensures
that these nested structures have the correct type(s) and key(s) where
appropriate for the QAPI context-free grammar.
(I replaced "the QAPI schema" with "a QAPI schema" as we have several;
and I wanted to hint at (somehow) that this processes configurable input
(i.e. "from disk") and not something indelibly linked.)
((What's wrong with "from disk?"))
PEP 8: You should use two spaces after a sentence-ending period in
multi- sentence comments, except after the final sentence.
Is this a demand?
+
+The QAPI schema expression language allows for syntactic sugar; this
suggest "certain syntactic sugar".
OK
+module also handles the normalization process of these nested
+structures.
+
+See `check_exprs` for the main entry point.
+
+See `schema.QAPISchema` for processing into native Python data
+structures and contextual semantic validation.
+"""
+
import re
from typing import (
Collection,
@@ -31,9 +48,10 @@
from .source import QAPISourceInfo
-# Deserialized JSON objects as returned by the parser;
-# The values of this mapping are not necessary to exhaustively type
-# here, because the purpose of this module is to interrogate that type.
+#: Deserialized JSON objects as returned by the parser.
+#:
+#: The values of this mapping are not necessary to exhaustively type
+#: here, because the purpose of this module is to interrogate that type.
First time I see #: comments; pardon my ignorance. What's the deal?
Sphinx-ese: It indicates that this comment is actually a block relating
to the entity below. It also means that I can cross-reference
`_JSONObject` in docstrings.
... which, because of the rewrite where I stopped calling this object an
Expression to avoid overloading a term, is something I actually don't
try to cross-reference anymore.
So this block can be dropped now, actually.
(Also: It came up in part one, actually: I snuck one in for EATSPACE,
and reference it in the comment for cgen. We cannot cross-reference
constants unless they are documented, and this is how we accomplish that.)
_JSONObject = Dict[str, object]
@@ -48,11 +66,29 @@
def check_name_is_str(name: object,
info: QAPISourceInfo,
source: str) -> None:
+"""Ensures that ``name`` is a string."""
PEP 257: The docstring [...] prescribes the function or method's effect
as a command ("Do this", "Return that"), not as a description;
e.g. don't write "Returns the pathname ...".
More of the same below.
Alright.
While we're here, then ...
I take this to mean that you prefer:
:raise: to :raises:, and
:return: to :returns: ?
And since I need to adjust the verb anyway, I might as well use "Check"
instead of "Ensure".
"""
Check that ``name`` is a string.
:raise: QAPISemError when ``name`` is an incorrect type.
"""
which means our preferred spellings should be:
:param: (and not parameter, arg, argument, key, keyword)
:raise: (and not raises, except, exception)
:var/ivar/cvar: (variable, instance variable, class variable)
:return: (and not returns)
Disallow these, as covered by the mypy signature:
:type:
:vartype:
:rtype:
if not
Re: [PATCH v4 16/19] qapi/expr.py: Add docstrings
John Snow writes:
> Signed-off-by: John Snow
> ---
> scripts/qapi/expr.py | 213 ++-
> 1 file changed, 208 insertions(+), 5 deletions(-)
>
> diff --git a/scripts/qapi/expr.py b/scripts/qapi/expr.py
> index 1869ddf815..adc5b903bc 100644
> --- a/scripts/qapi/expr.py
> +++ b/scripts/qapi/expr.py
> @@ -1,7 +1,5 @@
> # -*- coding: utf-8 -*-
> #
> -# Check (context-free) QAPI schema expression structure
> -#
> # Copyright IBM, Corp. 2011
> # Copyright (c) 2013-2019 Red Hat Inc.
> #
> @@ -14,6 +12,25 @@
> # This work is licensed under the terms of the GNU GPL, version 2.
> # See the COPYING file in the top-level directory.
>
> +"""
> +Normalize and validate (context-free) QAPI schema expression structures.
> +
> +After QAPI expressions are parsed from disk, they are stored in
> +recursively nested Python data structures using Dict, List, str, bool,
> +and int. This module ensures that those nested structures have the
> +correct type(s) and key(s) where appropriate for the QAPI context-free
> +grammar.
"from disk"? Perhaps something like "QAPISchemaParser parses the QAPI
schema into abstract syntax trees consisting of dict, list, str, bool
and int nodes." This isn't quite accurate; it parses into a list of
{'expr': AST, 'info': INFO}, but that's detail.
PEP 8: You should use two spaces after a sentence-ending period in
multi- sentence comments, except after the final sentence.
> +
> +The QAPI schema expression language allows for syntactic sugar; this
suggest "certain syntactic sugar".
> +module also handles the normalization process of these nested
> +structures.
> +
> +See `check_exprs` for the main entry point.
> +
> +See `schema.QAPISchema` for processing into native Python data
> +structures and contextual semantic validation.
> +"""
> +
> import re
> from typing import (
> Collection,
> @@ -31,9 +48,10 @@
> from .source import QAPISourceInfo
>
>
> -# Deserialized JSON objects as returned by the parser;
> -# The values of this mapping are not necessary to exhaustively type
> -# here, because the purpose of this module is to interrogate that type.
> +#: Deserialized JSON objects as returned by the parser.
> +#:
> +#: The values of this mapping are not necessary to exhaustively type
> +#: here, because the purpose of this module is to interrogate that type.
First time I see #: comments; pardon my ignorance. What's the deal?
> _JSONObject = Dict[str, object]
>
>
> @@ -48,11 +66,29 @@
> def check_name_is_str(name: object,
>info: QAPISourceInfo,
>source: str) -> None:
> +"""Ensures that ``name`` is a string."""
PEP 257: The docstring [...] prescribes the function or method's effect
as a command ("Do this", "Return that"), not as a description;
e.g. don't write "Returns the pathname ...".
More of the same below.
> if not isinstance(name, str):
> raise QAPISemError(info, "%s requires a string name" % source)
>
>
> def check_name_str(name: str, info: QAPISourceInfo, source: str) -> str:
> +"""
> +Ensures a string is a legal name.
I feel "legal" is best reserved to matters of law. Suggest "valid QAPI
name".
More of the same below.
> +
> +A legal name consists of ascii letters, digits, ``-``, and ``_``,
ASCII
> +starting with a letter. The names of downstream extensions are
> +prefixed with an __com.example_ style prefix, allowing ``.`` and
> +``-``. An experimental name is prefixed with ``x-``, following the
> +RFQDN if present.
I get that "an _com.experimental style prefix" and "the RFQDN" mean the
same thing, but can we make this clearer? Perhaps
A valid name consists of ascii letters, digits, ``-``, and ``_``,
starting with a letter. It may be prefixed by a downstream
prefix of the form __RFQDN_, or the experimental prefix ``x-``.
If both prefixes are present, the __RFQDN_ prefix goes first.
I'm clueless on proper use of `` in doc strings. Can you educate me?
> +
> +A legal name cannot start with ``q_``, which is reserved.
> +
> +:param name: Name to check.
> +:param info: QAPI source file information.
Suggest to say "QAPI schema source information", or maybe "QAPI schema
source file information".
> +:param source: Human-readable str describing "what" this name is.
Could mention it's for use in error messages, but we have many similar
parameters all over the place, so this would perhaps be more tiresome
than helpful. Fine as is.
> +
> +:return: The stem of the valid name, with no prefixes.
> +"""
> # Reserve the entire 'q_' namespace for c_name(), and for 'q_empty'
> # and 'q_obj_*' implicit type names.
> match = valid_name.match(name)
> @@ -62,6 +98,12 @@ def check_name_str(name: str, info: QAPISourceInfo,
> source: str) -> str:
>
>
> def check_name_upper(name: str, info: QAPISourceInfo, source: str) -> None:
> +"""
> +Ensures a
