JP-2299: Change residual fringe step reference models to FringeFreqModel #6385
Conversation
Codecov Repo 8000 rt
@@ Coverage Diff @@
## master #6385 +/- ##
==========================================
- Coverage 77.59% 77.25% -0.35%
==========================================
Files 408 409 +1
Lines 34895 35070 +175
==========================================
+ Hits 27077 27092 +15
- Misses 7818 7978 +160
*This pull request uses carry forward flags. Click here to find out more.
Continue to review full report at Codecov.
|
CHANGES.rst
Outdated
| @@ -2020,6 +2020,8 @@ residual_fringe | |||
| - Fixed an error when a filename was give as the input to apply the residual fringe correction [#6349] | |||
|
|
|||
| - Updated residual fringe reference data model to support new delivery of reference files [#6357] | |||
|
|
|||
| - Changed the model for reference file from ResidualFringeModel to FringeFreq [#6385] | |||
There was a problem hiding this comment.
Copy: extra space between "for" and "reference", possible word missing?
There was a problem hiding this comment.
How about "Changed reference file model name from ResidualFringeModel to FringeFreqModel"
| @@ -29,6 +28,9 @@ class ResidualFringeStep(Step): | |||
|
|
|||
| def process(self, input): | |||
|
|
|||
| self.transmission_level = 80 # sets the transmission level to use in the regions file | |||
There was a problem hiding this comment.
Slightly cleaner code would be to set this as a class-level attribute. Easier to override, if necessary, by subclasses, etc. Example:
class ResidualFringeStep:
transmission_level = 80 # sets the transmission level to use in the regions file
...
CHANGES.rst
Outdated
| @@ -2020,6 +2020,8 @@ residual_fringe | |||
| - Fixed an error when a filename was give as the input to apply the residual fringe correction [#6349] | |||
|
|
|||
| - Updated residual fringe reference data model to support new delivery of reference files [#6357] | |||
|
|
|||
| - Changed reference file model name from ResidualFringeModel to FringeFreq [#6385] | |||
There was a problem hiding this comment.
This new entry is waaaay down in the change log, in the section for Cal 0.15.0. We're way beyond that. This should be up at the top in the section for 1.4.0.
There was a problem hiding this comment.
Oh, and the new model name is FringeFreqModel, not FringeFreq
|
I moved the change entry to datamodels and the correct version version of the pipeline. It was more a data models issue rather than residual_fringe change. At least I think it should go data models changes ? |
| @@ -75,6 +75,8 @@ datamodels | |||
| and regions files. Fix an incorrect comment on | |||
| FilteroffsetModel. [#6362] | |||
|
|
|||
| - Changed reference file model name from ResidualFringeModel to FringeFreq [#6385] | |||
There was a problem hiding this comment.
I agree it's appropriate for this to go in the datamodels section. But the new name shown here is still incorrect: should be FringeFreqModel
…del (spacetelescope#6385) * Change residual fringe step reference models to FringeFreqModel * Update change log * fix import of FringeFreq * flake8 fix * update change log * moved change log entry to correct version
Closes #6381
Resolves JP-2299
Description
To be consistent with reference type the ResidualFringeModel was changed to FringeFreqModel. In addition, the argument 'transmission_level' was removed from options. This should not be something the user can change.
Checklist