-
Notifications
You must be signed in to change notification settings - Fork 37
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
HLA-1110: aperture keyword from poller to header #1683
HLA-1110: aperture keyword from poller to header #1683
Conversation
…rom_poller_to_header_10_10_23
Still need to test inputs as a list, as opposed to polller file, and testing the MVMs. |
Codecov ReportAttention:
... and 36 files with indirect coverage changes 📢 Thoughts on this report? Let us know!. |
Since your waiting on me @mdlpstsci , I'll include you in my current problem-solving. My issue right now is a couple SVM unit tests. It looks like gather_output_data is not working as intended, and not finding the test data that should be there. Any thoughts?
|
@mdlpstsci Actually it might be the fact that the test is testing full poller files, which I haven't tested yet. |
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.
Just a caution: For complex code, it is good to keep changes straightforward. I would not have inserted the new "aperture" value in the middle of the string which gets split up into tokens.
Please update any and all comment strings throughout the poller_utils.py module to include the new information. Again, I would use a different word than just "aperture" to differentiate it from the original FITS RAW keyword value.
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 should also add an entry in the CHANGELOG.
@s-goldman It does not look like the output is on Artifactory. So...I would get the dataset and process it with the original code, keeping all the output. I would then process it with the new code, and see if the differences are valid. |
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.
Important documentation changes.
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.
Ready to go!
Resolves HLA-1110
Closes #1661
This PR adds the aperture keyword to HAPProduct object as well as its child products (TotalProduct, FilterProduct, ExposureProduct). The code changes how the poller file data is used.
Previously the code either 1) used the poller file filenames and pulled the other relevant data from the header keywords 2) used full poller files with all of the relevant data. We have now implemented a third option where if the poller file has two columns, it is assumed that the second column is the aperture keyword. This code will then update the aperture keyword in the output SVM data for only WFPC2.
Checklist for maintainers
CHANGELOG.rst
within the relevant release sectionHow to run regression tests on a PR