Issues/47 support xtce unitset#258
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
==========================================
- Coverage 94.85% 94.80% -0.05%
==========================================
Files 48 48
Lines 3885 3892 +7
==========================================
+ Hits 3685 3690 +5
- Misses 200 202 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 similar comment
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #258 +/- ##
==========================================
- Coverage 94.85% 94.80% -0.05%
==========================================
Files 48 48
Lines 3885 3892 +7
==========================================
+ Hits 3685 3690 +5
- Misses 200 202 +2 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
medley56
left a comment
medley56
left a comment
There was a problem hiding this comment.
Nice. One test coverage request.
| if isinstance(self.unit, tuple): | ||
| unit_elements = [elmaker.Unit(u) for u in self.unit] | ||
| param_type_element.append(elmaker.UnitSet(*unit_elements)) | ||
| else: | ||
| param_type_element.append(elmaker.UnitSet(elmaker.Unit(self.unit))) | ||
|
|
There was a problem hiding this comment.
Add a unit test that covers these lines. This method is used when we serialize XML so it should live along side the other tests for that functionality.
There was a problem hiding this comment.
Pull request overview
Adds support for parsing and serializing multiple <xtce:Unit> elements within a <xtce:UnitSet>, replacing the previous NotImplementedError path. ParameterType.get_units now returns str for a single unit, tuple[str, ...] for compound units, or None. Serialization in both the base ParameterType.to_xml and EnumeratedParameterType.to_xml handles the tuple case.
Changes:
get_unitsreturnsstr | tuple[str, ...] | Noneinstead of raising on multiple units.to_xml(base and enumerated) serializes each element of a tuple as a separate<Unit>child.- Updated type hints and docstrings on
ParameterType,StringParameterType,BinaryParameterType,BooleanParameterType, andEnumeratedParameterType.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
space_packet_parser/xtce/parameter_types.py |
Adds tuple-based compound unit support in parsing and serialization; widens unit parameter types and docstrings. |
tests/unit/test_xtce/test_parameter_types.py |
Adds round-trip tests with multiple <xtce:Unit> entries for Integer, Float, and Binary parameter types. |
| ( | ||
| f""" | ||
| <xtce:BinaryParameterType xmlns:xtce="{XTCE_1_2_XMLNS}" name="TEST_PARAM_Type"> | ||
| <xtce:UnitSet> | ||
| <xtce:Unit>m</xtce:Unit> | ||
| <xtce:Unit>s</xtce:Unit> | ||
| </xtce:UnitSet> | ||
| <xtce:BinaryDataEncoding> | ||
| <xtce:SizeInBits> | ||
| <xtce:FixedValue>128</xtce:FixedValue> | ||
| </xtce:SizeInBits> | ||
| </xtce:BinaryDataEncoding> | ||
| </xtce:BinaryParameterType> | ||
| """, | ||
| parameter_types.BinaryParameterType( | ||
| name="TEST_PARAM_Type", | ||
| unit=("m", "s"), | ||
| encoding=encodings.BinaryDataEncoding(fixed_size_in_bits=128), | ||
| ), | ||
| ), |
Summary
Adds support for multiple
<xtce:Unit>elements within a<xtce:UnitSet>.Previously, only a single unit was supported. This change allows compound units to be
parsed and serialized in accordance with the XTCE specification.
Changes
ParameterType.get_unitsto return:strfor a single unittuple[str, ...]for multiple unitsto_xmlto correctly serialize multiple<Unit>elementsTesting
<xtce:Unit>support in:IntegerParameterTypeFloatParameterTypeBinaryParameterType