roman-corgi / corgidrp

Data Reduction Pipeline for the Roman Coronagraph Instrument
BSD 3-Clause "New" or "Revised" License
5 stars 4 forks source link

nonlinearity, trap-pump, and astrometric calibration products have unnecessary DQ header #244

Open kjl0025 opened 3 weeks ago

kjl0025 commented 3 weeks ago

Describe the bug A DQ header is created for the nonlinearity and trap-pump calibration products. However, the array values in the nonlinearity array are relative gain values for specific EM gain-DN combinations (not pixel data), and the array values for trap-pump are likewise not pixel data. So I don't think we need a DQ array here. Its presence doesn't hurt anything, but a user new to corgidrp may be confused to find this HDU data in the calibration product that doesn't have any meaning.

To Reproduce Steps to reproduce the behavior: For example, run an e2e test or unit test that creates a the calibration product.

Expected behavior An array of zeros should be produced for the 3rd extension header (the default DQ header).

corgidrp version: Main branch, v1.0

Additional context Should be a simple fix to remove the DQ header from the NonlinCalibration and TrapCalibration classes.

kjl0025 commented 3 weeks ago

Same applies for the astrometric calibration product.

semaphoreP commented 3 weeks ago

Are you saying the current DQ frame isn't an array of all 0's, or are you requesting the DQ extension to be removed completely.

Removing it may make it harder to subclass these data classes from the Image subclass, so I'm inclined to keep the DQ extension for simplicity.

kjl0025 commented 2 weeks ago

I was suggesting removing the DQ extension completely to avoid confusion. But I agree with you that it would be simpler to keep it, which would ensure good compatibility with Image. I don't have strong feelings on this one, just making folks aware of it. Maybe we could put a comment in the headers for these calibration classes that the DQ is not meaningful?

semaphoreP commented 2 weeks ago

Sure, sounds good to me.

kjl0025 commented 2 weeks ago

Cool, I'll add a comment to the relevant headers for my next PR.