Skip to content

Conversation

greglucas
Copy link
Collaborator

This is a major refactor, breaking the xtcedef.py module into separate entities.

I went with:

  • calibrators
  • encodings
  • exceptions
  • matches (comparisons? other ideas for this?)
  • packets (contains the sequence container and packet data definitions, would "containers" be a better module name?)
  • parameters (contains the Parameter (only one) and ParameterTypes)

I'm happy to move things around more/less too, just let me know your preferences. There is some fighting going on with circular imports depending on how we structure this, so we do need to make sure not to couple the modules too closely.

closes #34

Checklist

  • Changes are fully implemented without dangling issues or TODO items
  • Deprecated/superseded code is removed or marked with deprecation warning
  • Current dependencies have been properly specified and old dependencies removed
  • New code/functionality has accompanying tests and any old tests have been updated to match any new assumptions
  • The changelog.md has been updated

@greglucas greglucas requested a review from medley56 as a code owner September 4, 2024 22:50
@medley56
Copy link
Member

medley56 commented Sep 5, 2024

I think I borked this PR by merging your Packet/PacketData unification PR. Sorry about that. This is going to be a pain if we're changing code inside the modules. After my MIL-1750A PR I'm done for a while. Can we merge that, rebase this, and then merge it?

@medley56
Copy link
Member

medley56 commented Sep 5, 2024

@greglucas I'll approve this as soon as you can resolve the rebase.

@greglucas
Copy link
Collaborator Author

@medley56, this should be good to go with a rebase now and I can do large find/replace to move things around pretty easily if we want to change any namespaces.

My two questions for name changes (better suggestions welcome too!):

  • packets.py -> containers.py ?
  • matches.py -> comparisons.py ? (matches was slightly annoying because you used matches as a variable name and so we were shadowing the imported module name. There is also a new match syntax in Python 3.10+ that we might want to try to avoid confusion with)

@medley56
Copy link
Member

medley56 commented Sep 6, 2024

@greglucas I like comparisons.py over matches.py. I think I like parsing.py or maybe parseables.py more than containers.py because that module contains the Parsable protocol and everything in there seems to be related to actually parsing data.

@medley56 medley56 merged commit 3744845 into lasp:main Sep 6, 2024
7 checks passed
Comment on lines +551 to +553
_supported_encodings = ['IEEE-754', 'MIL-1750A']

def __init__(self, size_in_bits: int, encoding: str = 'IEEE-754',
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I believe these string restrictions are incorrect. They should be 'IEEE754' and 'MILSTD_1750A', rather than 'IEEE-754' and 'MIL-1750A'. Here's the enumerated value list for FloatEncodingType from lines 3886-3895 of the XTCE schema document:

<simpleType name="FloatEncodingType">
    <restriction base="string">
        <enumeration value="IEEE754_1985"/>
        <enumeration value="IEEE754"/>
        <enumeration value="MILSTD_1750A"/>
        <enumeration value="DEC"/>
        <enumeration value="IBM"/>
        <enumeration value="TI"/>
    </restriction>
</simpleType>

The above also agrees with the list in section 4.3.2.2.5.7.1 of the element description document.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Refactor xtcedef.py into sub modules
3 participants