Add type annotations #207
Add type annotations #207
Conversation
JonathonReinhart
force-pushed
the
add-typing
branch
3 times, most recently
from
7d3564a
to
eef8a44
Compare
JonathonReinhart
force-pushed
the
add-typing
branch
2 times, most recently
from
d85908d
to
3daf5fb
Compare
This also necessitated a few refactors: - Replace __wrap_docker_exec() with straightforward implementations - Move error message into DockerExecuteError.__init__ - Refactor out common behavior into _get_image_config()
Also, change git_describe() to return a GitDescribe dataclass result.
Other notable changes: - ScubaAlias, ScubaVolume are now frozen dataclass classes - Added Loader._rooted_loader to satisfy requirement that yaml.load's second parameter must be a Type - Added _get_str() - Added 'default' parameter to _get_typed_val()
Notable changes: - Made ScubaContext a dataclass with non-optional members. This required significant changes in ScubaContext.process_command(), since all values must be known when the object is constructed, and not default to None to be updated later in the function. - ScubaContext.process_command() image and shell arguments were renamed to add _override to clarify their behavior. Notable gaps: - open_scubadir_file() returns Any because I couldn't figure out how to correctly annotate a TextIO object with an additional container_path attribute.
Move argcomplete stub to its own module.
JonathonReinhart
force-pushed
the
add-typing
branch
from
3daf5fb
to
d8056ad
Compare
There was a problem hiding this comment.
I am not very familiar with Python type annotations, so I'm probably not the best person to check the correctness of the changes in this PR w.r.t. typing. However, all of the non-typing code changes look reasonable to me and since the tests are all still passing I'll conclude that there were no functionality regressions. There are still quite a few TODOs, but since they all concern typing I am comfortable with merging them into main.
Even though I'm not very experienced with the type annotations, I'm very glad to see that Scuba is moving to use them. I think this will make future modifications to Scuba by new developers easier (especially the code that parses and assembles config files) 👍🏻
|
Thanks @xanarin for taking a look.
I used to be slightly anti-typing since it felt like it went against the dynamic nature of Python. But then I worked in a codebase that required it, and learned to appreciate it.
Yep, exactly. #201 was the impetus for this. I had to chase a reference through 5 layers of function calls to figure out what its type was. |
This adds PEP 484 type annotations to all files in the
scuba/package, and enables type checking in CI via mypy.Fixes #204.