Step function: correct_bad_pixels

roman-corgi / corgidrp

Data Reduction Pipeline for the Roman Coronagraph Instrument

4 stars 2 forks source link

Step function: correct_bad_pixels #93

Closed hsergi closed 20 hours ago

hsergi commented 4 weeks ago

This PR covers the use of BadPixelMap to correct bad pixels in the data. UT goes over agreed sub-tests, including CR.

hsergi commented 2 weeks ago

@maxwellmb: As for the DQ doc string, I have: "dq (np.array): 2-D data quality, 0: good. Other values track different causes for bad pixels and other pixel-level effects in accordance with the DRP implementation document."

maxwellmb commented 6 days ago

@hsergi, please resolve the conflicts and then I'll take a look at this.

hsergi commented 6 days ago

@maxwellmb . I think I addressed all the comments. Outstanding ones: doc string for dq and using bitwise_or only. There was one more that I may misunderstand: bits are already set to DQ values before image data are replaced by NaN. I think it's fine as-is. If I am not getting the point, can you please tell me what it is missing exactly? The full comment is above.

hsergi commented 5 days ago

@maxwellmb: I think I have addressed your comment about setting the bit of "2" to 1 in both the correction function (L195) and the test function from: L67 to L79

maxwellmb commented 3 days ago

Alright, a few last minor thoughts:

You can delete the "MMB thoughts" now that things are implemented.
It would be great to make the docstring header a bit more descriptive (e.g. clarify how bad pixels are identified and what we do to replace the).
The "Returns" section says we're replacing by 0s, but at the moment its NaNs.
The description of the bp_mask input should instead say that this is a calibration file, not a bad pixel map for a specific frame.

Also, there are still merge conflicts that you need to resolve.

Otherwise we're good to go!