-
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
Use multiclass of uint32 and Enum rather than IntEnum #402
Use multiclass of uint32 and Enum rather than IntEnum #402
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #402 +/- ##
==========================================
- Coverage 97.56% 97.42% -0.14%
==========================================
Files 30 37 +7
Lines 2788 3451 +663
==========================================
+ Hits 2720 3362 +642
- Misses 68 89 +21 ☔ View full report in Codecov by Sentry. |
e500333
to
50b193f
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.
This looks good to me - let's wait until the mid-build release and then do a regtest with it as a sanity check.
61d0ac8
to
b837af0
Compare
This is currently blocked by spacetelescope/romancal/#1480, which makes a small change to cast back to python integer due to use of one of the methods that numpy integers do not replicate from python integers. |
b837af0
to
daa1930
Compare
This passes regression tests https://github.com/spacetelescope/RegressionTests/actions/runs/11670565333 |
daa1930
to
0caaf7b
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
…t32 rather than python int numpy/numpy#27540 (comment)
0caaf7b
to
57449fe
Compare
As part of the discussion in numpy/numpy#27540 about an issue related to the dq flags, it was suggested to instead do a multiclass of
np.uint32
andEnum
rather thanIntEnum
(which under the hood is a multiclass ofint
andEnum
). This is so that we prevent any mistakes related to choosing invalid integers underuint32
.Tasks
roman_datamodels
tests.docs/
page.no-changelog-entry-needed
.)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types).romancal
regression test (https://github.com/spacetelescope/RegressionTests/actions/workflows/romancal.yml) with this branch installed ("git+https://github.com/<fork>/rad@<branch>"
).News fragment change types:
changes/<PR#>.feature.rst
: new featurechanges/<PR#>.bugfix.rst
: fixes an issuechanges/<PR#>.doc.rst
: documentation changechanges/<PR#>.removal.rst
: deprecation or removal of public APIchanges/<PR#>.misc.rst
: infrastructure or miscellaneous change