-
Notifications
You must be signed in to change notification settings - Fork 16
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
CoDICE decompression algorithms for science data #278
CoDICE decompression algorithms for science data #278
Conversation
571521a
to
1682eae
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks great! Especially the comments and tests.
… codice utils folder to store some utils code akin to swe
1682eae
to
acb9487
Compare
e6d6407
to
cbfea18
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
looks great!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This code was weirdly pleasant to look at lol everything looks great!
decompressed_value : int | ||
The 24- or 32-bit decompressed value | ||
""" | ||
if algorithm == CoDICECompression.NO_COMPRESSION: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You could pass a function into this method instead of this if/else table. So, you could pass in _apply_lossy_a
as algorithm
and then this whole table could just be algorithm(decompress)
. Not sure if that makes sense for the rest of your code, but it could be an interesting option to reduce the number of functions and get rid of that enum.
https://stackoverflow.com/questions/1349332/python-passing-a-function-into-another-function
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left one suggestion that probably isn't a good match for this piece of code, but might help with later development based on this style
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
minor nit on the xfail/raises test. But looks good to me!
… codice utils folder to store some utils code akin to swe
cbfea18
to
e8921ab
Compare
…sing into codice-decompression
… assigning compression algorithm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Beautiful work. Everything looks great. I thought your last commit was a good catch. I think its ready to merge, and at a good spot whenever updates need to happen. (hopefully not anytime soon or too monstrous of changes)
…dice-decompression CoDICE decompression algorithms for science data
Change Summary
Overview
This PR implements the six possible CoDICE decompression algorithms for science data, with supporting unit tests. The algorithms are:
This PR closes #249 , however there will likely need to be some tweaks down the line to get this working with the greater CoDICE L0-L1A decom algorithm
New Dependencies
None
New Files
imap_processing/codice/l0/decompress_codice.py
imap_processing/codice/tests/test_decompress_codice.py
decompress_codice.py
Deleted Files
None
Updated Files
imap_processing/codice/l0/__init__.py
Testing
test_decompress_codice.py
contains a unit test for each of the 6 algorithms using an input value of234
(I picked this randomly), as well as an attempt to use an unsupported algorithm, which should result in a decompressed value ofNone
.