-
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
SWAPI Quality Flag #805
SWAPI Quality Flag #805
Conversation
Please update this page when you have a chance: |
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! I have a few questions/comments.
@@ -79,7 +79,7 @@ def test_decompress_count(): | |||
# compressed + no-overflow, compressed + overflow, no compression | |||
raw_values = np.array([[12, 0xFFFF, 12]]) | |||
compression_flag = np.array([[1, 1, 0]]) | |||
expected = np.array([[12 * 16, -1, 12]]) | |||
expected = np.array([[12 * 16, np.iinfo(np.int32).max, 12]], dtype=np.int32) | |||
returned_value = decompress_count(raw_values, compression_flag) | |||
np.testing.assert_array_equal(returned_value, expected) | |||
|
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.
It would be helpful to have one test that shows the flags set in dataset["swp_flags"].
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.
for sure! I will add a test for this.
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.
Laura, actually I am going to wait to add the test and do this in future PR if you don't mind. Right now, all flags are set to 0 but SWAPI will provide me some useful data that will have some good flags set. I will add the test with that change.
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.
I can't say much to the SWAPI DQ flags themselves, but the code changes look good to me!
bc38e92
to
c051959
Compare
I am going to merge this since there was not change request. |
Change Summary
Overview
Uses Quality flags class to set SWAPI quality flags. Closes #707
Updated Files
Testing
update test based on above changes