NASA-AMMOS / NASA-AMMOS/AIT-Core

Rethink Dtype class hierarchy

Open
#472 0 comments 0 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Dominant language
Python
Stars
56
Forks
35
Avg merge
1d 20h
Merged PRs (30d)
3

Description

We should rethink the way we're structuring our classes in dtype.py. Currently we have everything inheriting from Primitive type and it's turned into a bit of a messy catchall for all sorts of stuff. The inheritance of Complex types from it ends up being, at best, meaningless since the vast majority overwrite the encode / decode methods. Things like Time40 ends up shoehorned in by setting a 4-byte primitive type and then manually overwriting the byte/bit counts to support the 5-byte type.

TLDR: PrimitiveType isn't great.

I think we'd be better served by throwing an abstract base class in there to mandate the API that we require. It would likely make the user-facing portion of it more coherent as well.

Contributor guide

No contributing guide indexed for this repository

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

Research direction

Start by reading dtype.py, especially PrimitiveType, the Complex types, and Time40. Map the required encode/decode API and the current byte/bit-count handling before proposing an abstract base class. Done means the hierarchy no longer uses PrimitiveType as a catchall, existing types retain coherent behavior, and the user-facing API is clearer.

Written by the indexing model from the issue text.

Assessment

Tech stack
python
Domain
backend
Issue type
Refactor
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.