spacetelescope / romancal

Python library to process science observations from the Nancy Grace Roman Space Telescope
https://roman-pipeline.readthedocs.io/en/latest/
Other
28 stars 28 forks source link

RCAL-798: Populate the l3 product individual_image_meta block #1216

Closed stscieisenhamer closed 2 months ago

stscieisenhamer commented 2 months ago

Resolves RCAL-789 Requires roman_datamodels PR#348

This PR addresses implements the filling of the MosaicImage meta.individual_image_meta block.

Checklist

stscieisenhamer commented 2 months ago

I have left this as draft because:

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 76.57%. Comparing base (b61e70b) to head (da44930). Report is 18 commits behind head on main.

:exclamation: Current head da44930 differs from pull request most recent head a6d72e5. Consider uploading reports for the commit a6d72e5 to get more accurate results

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #1216 +/- ## ========================================== - Coverage 79.05% 76.57% -2.49% ========================================== Files 116 116 Lines 7994 7999 +5 ========================================== - Hits 6320 6125 -195 - Misses 1674 1874 +200 ``` | [Flag](https://app.codecov.io/gh/spacetelescope/romancal/pull/1216/flags?src=pr&el=flags&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | Coverage Δ | | *Carryforward flag | |---|---|---|---| | [nightly](https://app.codecov.io/gh/spacetelescope/romancal/pull/1216/flags?src=pr&el=flag&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | `62.78% <ø> (ø)` | | Carriedforward from [b61e70b](https://app.codecov.io/gh/spacetelescope/romancal/commit/b61e70beb5ff467d8eea5e722b19c2c197b39515?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) | *This pull request uses carry forward flags. [Click here](https://docs.codecov.io/docs/carryforward-flags?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=spacetelescope) to find out more.

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

schlafly commented 2 months ago

Thanks Jonathan. Do you have an L3 file you can point me to that I can take a look at?

stscieisenhamer commented 2 months ago

An example is found here

/grp/roman/eisenhamer/rcal-798-l3meta/roman_l3_metacheck.asdf

Note: This is saved from the test_l3_wcsinfo test because that test actually runs resample.

schlafly commented 2 months ago

Okay, I see, the dictionary issue is from meta.ref_file.crds, which seems okay. If we wanted to do that better we would do some kind of dictionary flattening first. Presumably tests are failing because the RDM change isn't in yet?

stscieisenhamer commented 2 months ago

Yes, the unknown dtype error is the error. Without the string-ing-fying, the dict is just an object which nothing likes. The ref_files.crds situation is the only one the currently exists, in which case flattening would be OK. But if more cases come up and flattening is attempted, one will need to ensure there is no key-name clashes.

schlafly commented 2 months ago

Okay, let's hold this until @PaulHuwe can take a look at it and the corresponding RDM PR, and then merge after the RDM update goes in.

It would also be good to either run regtests or manually grab a couple cal files from artifactory and run the HLP on them with this PR, just in case the real exposure metadata is different enough from the filler metadata to cause issues. I don't think I expect that, though.

stscieisenhamer commented 2 months ago

regression test

stscieisenhamer commented 2 months ago

new regression test

stscieisenhamer commented 2 months ago

Latest relevant regtest. Differences found are expected.

Believe all is now clear and ready for final review.