Jp 3861 refpix style #9234
Jp 3861 refpix style #9234
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9234 +/- ##
==========================================
- Coverage 73.66% 69.56% -4.11%
==========================================
Files 368 367 -1
Lines 36393 35585 -808
==========================================
- Hits 26809 24754 -2055
- Misses 9584 10831 +1247 ☔ View full report in Codecov by Sentry. |
|
|
||
| return data0 | ||
|
|
||
|
|
||
| def fft_interp_norm(dd0, mask0, row, hnorm, hnorm1, ny, ngroups, aa, n_iter_norm): | ||
|
|
||
| """Do something FFT related.""" |
There was a problem hiding this comment.
Was this the line you mentioned at stand-up? If so just flagging this so Melanie has a chance to look at it
There was a problem hiding this comment.
Hmm, this is obscure, isn't it? From the comments in fill_bad_regions, and some inspection of the code and inputs, it looks like it is doing an iterative filtering in FFT space of the normal pixels in each group.
Here's my best guess at the parameter values:
dd0: ndarray, Data array containing all groups, updated in place
mask0: ndarray, Mask for pixels to filter, with dimensions ny x nrow. 1 means use the pixel, 0 means do not use it.
row: int, Row size. Computed from the number of science pixels, reference pixels and padding in an amplifier.
hnorm: ndarray, Array of column indices for normal pixels.
hnorm1: ndarray, Shifted index values for normal pixels.
ny: int, Y size of data array.
ngroups: int, Number of groups.
aa: ndarray, Filter to apply.
n_iter_norm: int, Number of filtering iterations.
| # | ||
| # Just flip back |
There was a problem hiding this comment.
can this comment be integrated into the docstring instead of having both?
There was a problem hiding this comment.
there are a few more in the lines below that this might apply to
| Integration number | ||
| group : int | ||
| Group number | ||
| """ | ||
| # | ||
| # The inverse is to rotate 180 degrees, then flip over the line Y=X | ||
| self.group = np.swapaxes(self.group[::-1, ::-1], 0, 1) | ||
| self.restore_group(integration, group) | ||
|
|
||
|
|
||
| class NRCA1Dataset(NIRDataset): |
There was a problem hiding this comment.
for all of these classes that inherit from a parent class and all have the same few functions defined, if the parent class also has the function defined, then Sphinx will automagically copy over the docstring from the parent class. In that case you could just use a noqa: statement to ignore the style checker for all of these repeated methods.
In this case, it doesn't look like NIRDataset has a dms_to_detector, dms_to_detector_dq, or detector_to_dms method. I'm not familiar enough with the intent of this code to know for sure if this suggestion makes sense, but a possible future improvement could be to make NIRDataset an abstract base class and require child classes to define those three methods. It looks like the similar class for MIRI does so below.
| flag that controls whether odd and even-numbered columns are | ||
| Parameters | ||
| ---------- | ||
| input_model : jwst.datamodels.model |
There was a problem hiding this comment.
| input_model : jwst.datamodels.model | |
| input_model : jwst.datamodels.JWSTDataModel |
| """ | ||
| Check that the correction is skipped for MIR subarray data. | ||
| """ | ||
| # For MIRI, no reference pixel correction is performed on subarray data | ||
| # No changes should be seen in the data arrays before and after correction |
There was a problem hiding this comment.
same as before, can these be both in the docstring?
Also as written currently it looks like this is a single-line docstring, in which case it should be on one line. Since this is in a file starting with test_, the style checker doesn't ensure this is the case.
| """ | ||
| Check that odd/even rows are applied when flag is set. | ||
| """ |
There was a problem hiding this comment.
Same here, and other instances below - single-line docstrings should be on one line.
I consider it less important in the tests, so feel free to ignore this, but since you went to the trouble of changing these I figure it can't hurt to try to get them to match as closely as possible with the rules in the rest of the repo
Addresses JP-3861
Closes #
This PR applies style rules for the refpix step files
Tasks
Build 11.3(use the latest build if not sure)no-changelog-entry-needed)changes/:echo "changed something" > changes/<PR#>.<changetype>.rst(see below for change types)docs/pageokify_regteststo update the truth filesnews fragment change types...
changes/<PR#>.general.rst: infrastructure or miscellaneous changechanges/<PR#>.docs.rstchanges/<PR#>.stpipe.rstchanges/<PR#>.datamodels.rstchanges/<PR#>.scripts.rstchanges/<PR#>.set_telescope_pointing.rstchanges/<PR#>.pipeline.rststage 1
changes/<PR#>.group_scale.rstchanges/<PR#>.dq_init.rstchanges/<PR#>.emicorr.rstchanges/<PR#>.saturation.rstchanges/<PR#>.ipc.rstchanges/<PR#>.firstframe.rstchanges/<PR#>.lastframe.rstchanges/<PR#>.reset.rstchanges/<PR#>.superbias.rstchanges/<PR#>.refpix.rstchanges/<PR#>.linearity.rstchanges/<PR#>.rscd.rstchanges/<PR#>.persistence.rstchanges/<PR#>.dark_current.rstchanges/<PR#>.charge_migration.rstchanges/<PR#>.jump.rstchanges/<PR#>.clean_flicker_noise.rstchanges/<PR#>.ramp_fitting.rstchanges/<PR#>.gain_scale.rststage 2
changes/<PR#>.assign_wcs.rstchanges/<PR#>.badpix_selfcal.rstchanges/<PR#>.msaflagopen.rstchanges/<PR#>.nsclean.rstchanges/<PR#>.imprint.rstchanges/<PR#>.background.rstchanges/<PR#>.extract_2d.rstchanges/<PR#>.master_background.rstchanges/<PR#>.wavecorr.rstchanges/<PR#>.srctype.rstchanges/<PR#>.straylight.rstchanges/<PR#>.wfss_contam.rstchanges/<PR#>.flatfield.rstchanges/<PR#>.fringe.rstchanges/<PR#>.pathloss.rstchanges/<PR#>.barshadow.rstchanges/<PR#>.photom.rstchanges/<PR#>.pixel_replace.rstchanges/<PR#>.resample_spec.rstchanges/<PR#>.residual_fringe.rstchanges/<PR#>.cube_build.rstchanges/<PR#>.extract_1d.rstchanges/<PR#>.resample.rststage 3
changes/<PR#>.assign_mtwcs.rstchanges/<PR#>.mrs_imatch.rstchanges/<PR#>.tweakreg.rstchanges/<PR#>.skymatch.rstchanges/<PR#>.exp_to_source.rstchanges/<PR#>.outlier_detection.rstchanges/<PR#>.tso_photometry.rstchanges/<PR#>.stack_refs.rstchanges/<PR#>.align_refs.rstchanges/<PR#>.klip.rstchanges/<PR#>.spectral_leak.rstchanges/<PR#>.source_catalog.rstchanges/<PR#>.combine_1d.rstchanges/<PR#>.ami.rstother
changes/<PR#>.wfs_combine.rstchanges/<PR#>.white_light.rstchanges/<PR#>.cube_skymatch.rstchanges/<PR#>.engdb_tools.rstchanges/<PR#>.guider_cds.rst