Yaml dataclass tests #19
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This mirrors the oresat0 and oresat0_5 tests.
This adds a series of tests for the conversion of yaml config files to their corresponding dataclass types, along with fixes to discovered errors. The main error is really one of yaml's fault. In yaml 1.1 bools are not only true/false, but also yes/no, y/n, and, specific to here, on/off. See https://yaml.org/type/bool.html for more. pyyaml will convert these types to python's bool: True or False. Where on and off are used in the configs, they are intended to be strings so quoting needs to be applied to keep the string value. As an added bonus dataclasses_json's from_dict() method is type aware, so when it gets a mixed key types dict, it will implicitly convert the now bool values to string "True" or "False", as that's what the CardConfig specifies for the relevant field. For example if you load c3.yaml into a CardConfig c then: c.objects[-2].generate_subindexes.value_descriptions == {'False': 0, 'boot': 1, 'True': 2, 'error': 3, 'not_found': 4, 'dead': 255} Where we would expect {'off': 0, ..., 'on': 2, ...}. The tests introduce the dacite package, which does a similar from_dict that dataclasses_json does, but is type aware and won't do implicit conversion, raising an exception instead. It even has a strict checking mode to ensure all entries are consumed, but using it will have to wait for a later time. An alternate solution I considered was switching to yaml 1.2, which defines bools as strictly true or false, but it turns out pyyaml doesn't yet support 1.2 despite it being out since 2009. In fact, it doesn't look like the pyyaml project is very alive any more. The last update was in 2020 and its ticket tracker is slowly fading out.
Such that it's able to pass `mypy --strict`. It looks like a lot but it's mostly just adding return type `-> None` to functions that don't have an explicit return. This started as completing some of the container types (`dict`, `list`) in `_yaml_to_od.py` to improve my readability of whats going on there, but then spiraled outwards. Changes which aren't just trivial type completion: - `Card` and `cards_from_csv` have been moved from `__init__.py` to avoid circular imports when using the `Card` type. - `remove_node_id()` in `gen_fw_files.py` had a strange type, but the weird bits were already being done outside the function, so I simplified it to `str -> str` (along with changes outside to make it work). - `unittest.TestCase` has `id()` as a member method, so assigning the OreSat ID was replacing it with an incompatible type. I've renamed our use from `id` to `oresatid`. - Type info for pyyaml is now available so I've pulled in that package and enabled checking for it. Canopen has type information as well but it doesn't look ready yet - some of the types are incorrect and they're not externally accessible anyway. It looks like the next version of canopen will probably have them. I used the escape hatch `Any` type in two areas: - The Loader/CLoader import from yaml wasn't passing muster. Ideally we never want just Loader though so this can be simplified in the future. - `register_subparser()` in scripts. The type passed to it is private to `argparse` (`_SubParserAction` technically) . It has no documented type beyond a thing which provides the `add_parser()` method.
ryanpdx
approved these changes
Feb 26, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds a couple of tests, some type information and introduces the
dacitepackage for converting from dicts to dataclasses.This shouldn't add any new behavior (beyond minor bug fixes - see the description of 4ff2a0c). The bulk of the changes are type information, especially adding
-> Noneto functions that don't have an explicitreturn. The type information involved is enough to get it to passmypy --strict, and prepares it for being PEP-561 compliant (specificallypy.typed). Making it actually fully compliant I felt was beyond the scope of this PR.