JP-1739: Remove references to stdatamodels.ndmodel #6179
Conversation
|
Regression test run with this branch and the stdatamodels branch that removes the module: https://plwishmaster.stsci.edu:8081/job/RT/job/eslavich-dev/90 |
Codecov Report
@@ Coverage Diff @@
## master #6179 +/- ##
==========================================
- Coverage 77.02% 76.65% -0.37%
==========================================
Files 402 402
Lines 34366 34694 +328
==========================================
+ Hits 26469 26596 +127
- Misses 7897 8098 +201
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report at Codecov.
|
| if ( | ||
| model.meta.hasattr('wcsinfo') | ||
| and model.meta.wcsinfo.hasattr('spectral_order') | ||
| and model.meta.wcsinfo.spectral_order is not None | ||
| ): |
There was a problem hiding this comment.
For any datamodel passed to photom, it will have a wcsinfo attribute, and if it does (because it includes wcsinfo.schema) then spectral_order will definitely be an attribute (with a value or None).
So this check might more simply be:
if model.meta.wcsinfo.spectral_order is not None:
do stuff@stscieisenhamer any reason you used model.meta.get('wcsinfo.spectral_order') instead of the above?
There was a problem hiding this comment.
I.e. if the input datamodel doesn't include the wcsinfo.schema, then the original code will fail with AttributeError as well. And if self.order has a default of None, why assign it and then have a check in the first place?
There was a problem hiding this comment.
I tried that first, but tests fail due to wcsinfo being None. I didn't look into why that might be.
There was a problem hiding this comment.
All datamodels should have a wcsinfo block, i.e. include that schema, or else they should not even make it to the photometric calibration step in stage 2 pipeline. So do we have unit tests that are testing for something that will never occur in the wild?
There was a problem hiding this comment.
Ah, it looks like this is to handle MultiSlitModel where the wcsinfo block is in
for slit in model.slits:
self.spectral_order = slit.meta.wcsinfo.spectral_orderSo this seems like a not great way to initialize the DataSet class, because the question becomes how is order determined (if not on __init__) for MultiSlitModel? Not the same as for the others.
Anyway, not worth debugging now.
* Remove references to stdatamodels.ndmodel * Fix hasattr check * Fix style issue * Add changelog entry
Addresses #5380 / JP-1739
Description
This PR removes references to the stdatamodels.ndmodel module which is being removed over in stdatamodels. I've also updated code that uses NDData interface methods.
Checklist
Tests
Documentation
Change log
Milestone
Label(s)