Conversation
| self._defects.append(self._check_value(value)) | ||
| self._normalize() | ||
| if reason: | ||
| self._defectsUnnormalized.append((self._check_value(value), reason)) |
There was a problem hiding this comment.
If no reason is given then they don't go into the unnormalized defects? Maybe it should default to reason UNKNOWN or something?
There was a problem hiding this comment.
So a defectsUnnormalized is a tuple? I think that is potentially confusing. Is it possible to define a DefectWithReason here ... or maybe a continueClass in meas_algorithms to extend Defect ... https://github.com/lsst/meas_algorithms/blob/main/python/lsst/meas/algorithms/interp.py. This could give a .reason attribute that defaults to either "UNKNOWN" or None; otherwise behaves like a defect (with a bbox) which will simplify things.
Thinking this through I think that a new DefectWithReason type class may be appropriate, could be here or in meas_algorithms.
|
|
||
| return values[:n] | ||
|
|
||
| def getReasonDict(self): |
There was a problem hiding this comment.
I would prefer if there were too methods here ... getReasonToBitMapping() and getBitToReasonMapping() or something like that?
Also this doesn't have a docstring.
| # Tests masking with reason is working properly | ||
| ccdImage = afwImage.MaskedImageF(250, 225) | ||
| # test 1. test mask according to a given reason | ||
| reason = 'HOT_PIXEL' |
There was a problem hiding this comment.
Do we have a standard list of defect reasons we want to define? And should they be HOT or HOT_PIXEL (isn't pixel implied?)
| with self.assertRaises(RuntimeError): | ||
| defects.maskPixelsReason(ccdImage.mask, reason) | ||
|
|
||
| # test 2. test mask plane with reasons is set properly |
Some notes. Support defect reason and masking helper. Add check defects unnormalized are existent. Add unit test defects reason. Support definition of mask plane by reason. Linting. Add unit test. Comment out reason per mask plane. Add followup ticket number.
3 participants
No description provided.