-
Notifications
You must be signed in to change notification settings - Fork 25
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
JP-2349: added v2/v3 columns to moving target position table schema #263
Conversation
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #263 +/- ##
=======================================
Coverage 66.21% 66.21%
=======================================
Files 101 101
Lines 5402 5402
=======================================
Hits 3577 3577
Misses 1825 1825 ☔ View full report in Codecov by Sentry. 🚨 Try these New Features:
|
Keeping this in draft form until SDP adds the new columns to the table (in level-1b processing), which likely won't happen until DMS B11.0. |
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
Note: this PR breaks some JWST regression tests, see: https://plwishmaster.stsci.edu:8081/job/RT/job/JWST-Developers-Pull-Requests/1662/testReport/junit/jwst.regtest/test_nircam_mtimage/_stable_deps__test_nircam_image_moving_target_kwds_with_mt_table_/ Will need to update our regression test data and truth data to accommodate new schema structure before the next stdatamodels release. |
It looks like this update to the moving target schema may be causing issues opening MAST files with JWST pipeline version 1.16. After updating the pipeline to version 1.16.0, when attempting to open a moving target uncal file as a datamodel, I now get the error ValueError: Column names don't match schema. Schema has {'mt_v3', 'mt_v2'}. Data has set() Is there a work around for this? Or do we need to use a previous pipeline version for moving targets for now? |
Hi @RyleighDavis , I've filed a workaround here: #329 If you're comfortable installing from the pull (or my branch), that should fix your issues. I think we should be able to get this merged and released in the next couple of days, pending any further issues discovered. |
Resolves JP-2349
Closes spacetelescope/jwst#6507
This PR addresses adding V2/V3 coordinates to the moving target position table schema, in order to provide position information for SLIT and COMPOUND aperture types.
Checklist
CHANGES.rst
(either inBug Fixes
orChanges to API
)