-
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
[SCSB-174] move DMS requirement <-> test correlations from @metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
#1399
Conversation
0d0e4da
to
b237cf8
Compare
@metrics_logger()
test decorators with test_requirements.json
filemetrics_logger
with test_requirements.json
file
Thanks for taking this on. Would you add a test that confirms the tests listed in the json file are found by pytest? That way if a future PR renames or removes a test we'll know before the metrics collection fails. |
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1399 +/- ##
==========================================
+ Coverage 78.46% 78.55% +0.08%
==========================================
Files 117 117
Lines 7866 7843 -23
==========================================
- Hits 6172 6161 -11
+ Misses 1694 1682 -12
*This pull request uses carry forward flags. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Can we hide the |
good idea, would you want to put it in |
Good idea! I think |
08329e1
to
cbeee14
Compare
I "approve" this but leave the final approvals up to @jglorioso-stsci and @schlafly. Thanks! |
967c5b0
to
cb9a258
Compare
metrics_logger
with test_requirements.json
file@metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
@metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
@metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
@metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
@metrics_logger
decorators to romancal/tests/dms_requirement_tests.json
dd2dd9d
to
f64cdcc
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.
Missing a few DMS requirements in test_ramp_fitting.py. Otherwise, looks good!
Resolves SCSB-174
consolidates all test requirement correlations into
romancal/tests/dms_requirement_tests.json
, obviating the need to insert log messages at test execution time withmetrics_logger
This PR (and its sister PRs spacetelescope/romanisim#146 and spacetelescope/crds#1064) blocks https://grit.stsci.edu/dmd/roman-metrics/-/merge_requests/21
Checklist
for a public change, added a towncrier news fragment in
changes/
echo "changed something" > changes/<PR#>.<changetype>.rst
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
updated relevant tests
updated relevant documentation
updated relevant milestone(s)
added relevant label(s)
ran regression tests, post a link to the Jenkins job below. How to run regression tests on a PR