-
Notifications
You must be signed in to change notification settings - Fork 21
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
Move dqflags
from romancal
to roman_datamodels
and make them enums.
#293
Move dqflags
from romancal
to roman_datamodels
and make them enums.
#293
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #293 +/- ##
==========================================
+ Coverage 97.48% 97.56% +0.07%
==========================================
Files 28 30 +2
Lines 2704 2788 +84
==========================================
+ Hits 2636 2720 +84
Misses 68 68 ☔ View full report in Codecov by Sentry. |
53e692b
to
e9cb25a
Compare
763c085
to
06083c0
Compare
c7f8d05
to
b600c23
Compare
This makes the flags immutable and enables `.` access. This still allows dictionary like access e.g. pixel["GOOD"]
directly and validate without issues
These are changes made by spacetelescope/romancal#981
for more information, see https://pre-commit.ci
b600c23
to
74d8aa8
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.
LGTM. Be sure to get approval on the RCAL companion ticket before merging this.
This looks good to me. It's good to have this in roman_datamodels rather than romancal so that people can use the flags without bringing in romancal as a dependency. |
… them enums. (spacetelescope#293)" This reverts commit 6833d01.
… them enums. (spacetelescope#293)" This reverts commit 6833d01.
When working on jump detection for
romancal
I noticed that thedqflags
were stored inromancal
instead ofroman_datamodels
. Thedqflags
being directly part of the pipeline instead of the datamodels package proved to be an issue for JWST as some tools wished to not have a direct dependency on the JWST pipeline simply to open and interpret existing output files, see spacetelescope/stdatamodels#134.This PR begins the process of moving the
dqflags
fromromancal
toroman_datamodels
by "duplicating" them intoroman_datamodels
. Note that at this time,romancal
will still be referencing its internally defined flags, an update to this effect can be made later toromancal
which is seamless to the development process.Additionally, this PR transitions the
dqflags
from using simple dictionaries to defining them using anEnum
as I discussed briefly with @schlafly. This adds two useful things:dict
is susceptible to the possibility of a module importing the dqflags updating a flag's value, which would change that value globally..
access to the flag values e.g.pixel.GOOD
while still retaining the dictionary like access e.g.pixel["GOOD"]
. The.
access is nice because it enables static checkers to "flag" when something like a typo in a flag's name occurs and code completion engine's to suggest the list of possible flags.Checklist
CHANGES.rst
under the corresponding subsection