-
Notifications
You must be signed in to change notification settings - Fork 28
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
rcal-949 Update file naming and asn header coords #1505
Conversation
f7addf1
to
05c23b5
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #1505 +/- ##
==========================================
- Coverage 76.26% 76.24% -0.02%
==========================================
Files 115 115
Lines 7643 7650 +7
==========================================
+ Hits 5829 5833 +4
- Misses 1814 1817 +3 ☔ View full report in Codecov by Sentry. |
8fe4621
to
f214883
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.
Thanks Dave. This looks good. Harry made a further update to the file names to remove the wfi string from the L3 products.
e.g.
rPPPPP_p_v0102010001001_274p63x31y81_f106_coadd.asdf
https://innerspace.stsci.edu/pages/viewpage.action?spaceKey=RDMSDOC&title=Roman+Science+File+Naming+Conventions+for+Files+in+the+Archive+Catalog
I think this just means re-removing the +instrument and a +sep.
be22fec
to
70e2c71
Compare
70e2c71
to
04de8c5
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 good to me.
I only left one suggestion that would make the code more maintainable.
romancal/associations/skycell_asn.py
Outdated
if product_type == "visit": | ||
pr_name = ( | ||
"v" | ||
+ parsed_visit_id["Execution"] | ||
+ parsed_visit_id["Pass"] | ||
+ parsed_visit_id["Segment"] | ||
+ parsed_visit_id["Observation"] | ||
) | ||
elif product_type == "daily": | ||
pr_name = ( | ||
"d" | ||
+ parsed_visit_id["Execution"] | ||
+ parsed_visit_id["Pass"] | ||
+ parsed_visit_id["Segment"] | ||
) | ||
elif product_type == "pass": | ||
pr_name = "p" + parsed_visit_id["Execution"] + parsed_visit_id["Pass"] | ||
elif product_type == "full": | ||
pr_name = "full" | ||
elif product_type == "user": | ||
pr_name = "user" | ||
else: | ||
pr_name = "unknown" | ||
|
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 think the code would be more concise and easier to maintain if we used a mapping instead of a conditional block here. Something along the lines of:
product_name_mapping = {
"visit": "v" + parsed_visit_id[1] + parsed_visit_id[...] + parsed_visit_id[n],
"daily": "d" + parsed_visit_id[1] + parsed_visit_id[...] + parsed_visit_id[n],
"pass": "p" + parsed_visit_id[1] + parsed_visit_id[...] + parsed_visit_id[n],
"full": "full",
"user": "user"
}
pr_name = product_name_mapping.get(product_type, "unknown")
450c042
to
49104fc
Compare
for more information, see https://pre-commit.ci
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 good! Thanks.
Resolves RCAL-949
Closes #1501
This PR updates the the file naming conventions and adds the orientation to the wcs keywords in the asn header.
Tasks
24Q4_B15
(use the latest build if not sure)no-changelog-entry-needed
)changes/
:echo "changed something" > changes/<PR#>.<changetype>.rst
(see below for change types)docs/
pagehttps://github.com/spacetelescope/RegressionTests/actions/runs/11783036917
okify_regtests
to update the truth filesnews fragment change types...
changes/<PR#>.general.rst
: infrastructure or miscellaneous changechanges/<PR#>.docs.rst
changes/<PR#>.stpipe.rst
changes/<PR#>.associations.rst
changes/<PR#>.scripts.rst
changes/<PR#>.mosaic_pipeline.rst
changes/<PR#>.patch_match.rst
steps
changes/<PR#>.dq_init.rst
changes/<PR#>.saturation.rst
changes/<PR#>.refpix.rst
changes/<PR#>.linearity.rst
changes/<PR#>.dark_current.rst
changes/<PR#>.jump_detection.rst
changes/<PR#>.ramp_fitting.rst
changes/<PR#>.assign_wcs.rst
changes/<PR#>.flatfield.rst
changes/<PR#>.photom.rst
changes/<PR#>.flux.rst
changes/<PR#>.source_detection.rst
changes/<PR#>.tweakreg.rst
changes/<PR#>.skymatch.rst
changes/<PR#>.outlier_detection.rst
changes/<PR#>.resample.rst
changes/<PR#>.source_catalog.rst