More type annotations #38
Conversation
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
These types are "consumed" by the lib but annoying to remember and type (with a keyboard) over and over again. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Generally, this would come from argparse but I don't want to pull in argparse all over the place. Rather, use a new Protocol type with just the methods we use in our CLI construction functions. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
I suppose mypy sees that block outside of the context manager as a path with no return statement and throws an error. I don't think that is quite right, but I am probably wrong and it isn't a huge change needed to quiet the error. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Setting disallow_incomplete_defs means that if we have an type annotation on one arg or the return type, all args and the return type must be annotated. I think this is a good default we can apply across all modules in the sambacc codebase. Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
Signed-off-by: John Mulligan <jmulligan@redhat.com>
phlogistonjohn
force-pushed
the
jjm-better-typing
branch
from
015de67
to
3042db4
Compare
There was a problem hiding this comment.
A general comment: PEP484, in The meaning of annotations requires __init__ to have -> None type hinting. Not all cases follow this rule.
| @@ -298,7 +312,9 @@ def _node_check(pnn: int, nodes_json: str, real_path: str) -> bool: | |||
| return True | |||
|
|
|||
|
|
|||
| def _node_update_check(json_data, nodes_json: str, real_path: str): | |||
| def _node_update_check( | |||
| json_data: dict[str, typing.Any], nodes_json: str, real_path: str | |||
There was a problem hiding this comment.
nodes_json is not used in this function. With type hinting, functions signatures become longer; maybe it would be better to avoid used arguments.
There was a problem hiding this comment.
Function is private so it should be straightforward.
There was a problem hiding this comment.
I started to try and remove this but it cascades through other layers. I am going to leave it for now with consistency with other function signatures. Next time I may need to make better use of keyword-only arguments for something like this.
Right. This was (is?) tricky for me when I started using annotations. My plan is to enable annotations required for |
phlogistonjohn
force-pushed
the
jjm-better-typing
branch
from
3042db4
to
26ed331
Compare
|
Rebased. |
|
Thanks!! |
To improve the type annotations usage in sambacc and hopefully catch more errors early the first step is to enable the option "disallow_incomplete_defs" in mypy. This disallows mixing typed and untyped parameters and return types on a single function call. It does not affect a function that has no annotations at all.
The first set of commits adds annotations to many parts of the code base. Then we enable the option in mypy to enforce it in the future (via pyproject.toml). Finally, we update a few straggling uses of typing.List and typing.Dict to the 'list' and 'dict' equivalents since our minimum version is python3.9.