JP-3773: Store logs from calibration pipeline in datamodel #8857
Conversation
tapastro
force-pushed
the
jp-3773-log-extension
branch
from
c7cf01a
to
51837d1
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #8857 +/- ##
==========================================
+ Coverage 61.79% 61.81% +0.01%
==========================================
Files 377 377
Lines 38834 38849 +15
==========================================
+ Hits 23999 24015 +16
+ Misses 14835 14834 -1 ☔ View full report in Codecov by Sentry. |
|
These logs appear to include all log messages regardless of level, e.g. This may be caused by the log setting that appears at the top of most jwst code files, but perhaps occurs in step |
|
I think this PR could benefit from some unit tests for the new behavior and documentation on what is contained in Looking at this PR (as it currently stands), Some questions I have are:
One issue I ran into was, if I run I'm trying to think of what a "minimally viable" version of this feature needs. I think what is in this PR is sufficient (with the known issues above) but tests and docs seem worthwhile. One final question, does it make sense to have this be "opt-in" for now perhaps adding a "save_cal_logs" to the spec and having it default to "False"? That way:
|
| if hasattr(result, 'cal_logs'): | ||
| tmpdict = result.cal_logs.instance | ||
| else: | ||
| tmpdict = dict() | ||
| tmpdict[self.class_alias] = '\n'.join(self._log_records) | ||
| result.cal_logs = tmpdict |
There was a problem hiding this comment.
| if hasattr(result, 'cal_logs'): | |
| tmpdict = result.cal_logs.instance | |
| else: | |
| tmpdict = dict() | |
| tmpdict[self.class_alias] = '\n'.join(self._log_records) | |
| result.cal_logs = tmpdict | |
| if not hasattr(result, 'cal_logs'): | |
| result.cal_logs = {} | |
| setattr(result.cal_logs, self.class_alias, self._log_records) |
Some code golf and switching the cal_log to a list of strings.
| if hasattr(result, 'cal_logs'): | ||
| tmpdict = result.cal_logs.instance | ||
| else: | ||
| tmpdict = dict() | ||
| tmpdict[self.class_alias] = '\n'.join(self._log_records) | ||
|
|
||
| for _, step in self.step_defs.items(): | ||
| if step.class_alias in tmpdict.keys(): | ||
| del tmpdict[step.class_alias] | ||
| result.cal_logs = tmpdict |
There was a problem hiding this comment.
| if hasattr(result, 'cal_logs'): | |
| tmpdict = result.cal_logs.instance | |
| else: | |
| tmpdict = dict() | |
| tmpdict[self.class_alias] = '\n'.join(self._log_records) | |
| for _, step in self.step_defs.items(): | |
| if step.class_alias in tmpdict.keys(): | |
| del tmpdict[step.class_alias] | |
| result.cal_logs = tmpdict | |
| if not hasattr(result, 'cal_logs'): | |
| result.cal_logs = {} | |
| for _, step in self.step_defs.items(): | |
| if hasattr(result.cal_logs, step.class_alias): | |
| delattr(result.cal_logs, step.class_alias) | |
| setattr(result.cal_logs, self.class_alias, self._log_records) |
Also switching cal_logs to a list of strings and removing tmpdict.
tapastro
changed the title
Resolves JP-3773
Closes #8855
This PR adds the log records during processing to a datamodel attribute
model.cal_logs. This attribute is a dictionary, with keys equal to steps and/or pipelines applied to the data, and values associated with those keys equal to the log output.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#>.fits_generator.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