DM-27103: Not all IsrCalib tests round trip calibrations #178
Conversation
| return False | ||
|
|
||
| for attr in self._requiredAttributes: | ||
| if getattr(self, attr) != getattr(other, attr): | ||
| attrSelf = getattr(self, attr) |
There was a problem hiding this comment.
The big change in this ticket is to make the eq method general, by being part of calibType, correct? I see that you deleted the eq in the PTC and BFK code. Were these the only calibrations that had a customized eq?
There was a problem hiding this comment.
Yes. Since they were doing essentially the same kind of checks, it made sense to collect them in the base class __eq__ so any future calibs will have a functional equivalence with no extra work.
| if 'CT_ERRORS' in coeffTable.columns: | ||
| inDict['coeffErr'] = coeffTable['CT_ERRORS'] | ||
| if 'CT_COUNTS' in coeffTable: | ||
| if 'CT_COUNTS' in coeffTable.columns: | ||
| inDict['coeffNum'] = coeffTable['CT_COUNTS'] | ||
| if 'CT_VALID' in coeffTable: | ||
| if 'CT_VALID' in coeffTable.columns: |
There was a problem hiding this comment.
Why now do you have to check coeffTable.columns instead of just coeffTable? Is this the only place where this needs to be changed?
There was a problem hiding this comment.
Without the change, astropy throws a
astropy/table/row.py:76: FutureWarning: elementwise == comparison failed and returning scalar instead; this will raise an error or perform elementwise comparison in the future. return self.as_void() == other warning that I found annoying. I don't know why checking inclusion requires an __eq__ run on the row, and didn't care to dig deeper. Checking the .columns attribute should have the same behavior, and as it isn't calling whatever the other style is calling, it probably runs infinitesimally faster.
- Did not set detectorId from an inherited detector. - Did not set ERROR, NUM, and VALID fields when inheriting from a detector. - Look for table names in astropy Table.columns to avoid an annoying warning.
This fixes the missing tests, and enhances the base class eq to reduce code duplication.