Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Yaml dataclass tests #19

Merged
merged 4 commits into from
Feb 26, 2024
Merged

Yaml dataclass tests #19

merged 4 commits into from
Feb 26, 2024

Commits on Feb 14, 2024

  1. Add oresat1 test

    This mirrors the oresat0 and oresat0_5 tests.
    ThirteenFish committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    2d50bc2 View commit details
    Browse the repository at this point in the history
  2. Ensure yaml matches dataclass types

    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.
    ThirteenFish committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    4ff2a0c View commit details
    Browse the repository at this point in the history
  3. Complete type annotations

    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.
    ThirteenFish committed Feb 14, 2024
    Configuration menu
    Copy the full SHA
    8c71027 View commit details
    Browse the repository at this point in the history

Commits on Feb 26, 2024

  1. Merge branch 'master' into yaml-dataclass-tests

    This incorporates the changes from PRs #16, #17, and #18. This branch
    was originally based on #16 before feedback.
    
    I don't believe this introduces any new behavior
    ThirteenFish committed Feb 26, 2024
    Configuration menu
    Copy the full SHA
    60917b8 View commit details
    Browse the repository at this point in the history