DM-33429: Add ability to do both serial and parallel overscan correction #219
Conversation
czwa
commented
Jul 13, 2022
czwa
commented
- Remove isrFunction intermediate for overscan correction.
- Add support for parallel overscan correction.
- Fix handling of column/row skipping, to ensure that overscan result is subtracted where needed.
- Consolidate unit tests for overscan correction in one location.
| self.metadata[f"FIT MEDIAN {amp.getName()}"] = qaMedian | ||
| self.metadata[f"FIT STDEV {amp.getName()}"] = qaStdev | ||
| self.metadata[f"FIT MEDIAN {amp.getName()}"] = overscanResults.overscanMean | ||
| self.metadata[f"FIT STDEV {amp.getName()}"] = overscanResults.overscanSigma |
There was a problem hiding this comment.
Why do they have to go in this task's metadata? Can't they go in the overscan task's metadata? In general, the overscan task should do everything to do with the overscan, and the ISR task should delegate everything to do with the overscan to the overscan task.
tests/test_isrFunctions.py
Outdated
| @@ -254,7 +254,7 @@ def test_illumCorrection(self): | |||
| with self.assertRaises(RuntimeError): | |||
| ipIsr.illuminationCorrection(self.mi, flatMi, 1.0, trimToFit=False) | |||
|
|
|||
| def test_overscanCorrection_isInt(self): | |||
| def notest_overscanCorrection_isInt(self): | |||
There was a problem hiding this comment.
Why have these been disabled? There's no note in the commit message, and it's not clear what "to check for Jenkins level failures" means.
Besides disabling the tests, there are other functional changes, which should probably be part of a different commit.
There was a problem hiding this comment.
This has been removed, as the coverage is handled by the test_overscanCorrection.py tests.
tests/test_overscanCorrection.py
Outdated
| statAfter = computeImageMedianAndStd(self.inputExp.image[self.amp.getRawDataBBox()]) | ||
| self.assertLess(statAfter[0], statBefore[0]) | ||
|
|
||
| def notest_overscanCorrection_isInt(self): |
There was a problem hiding this comment.
These tests are still disabled.
There was a problem hiding this comment.
Coverage now provided by other tests and this method removed.
czwa
force-pushed
the
tickets/DM-33429
branch
2 times, most recently
from
84b3b45
to
73e2235
Compare
* Remove isrFunction code for overscans. * Move overscan bbox handling to overscan task. * Add support for parallel overscan correction.
