JP-3858-exp_to_source #9201
JP-3858-exp_to_source #9201
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #9201 +/- ##
==========================================
- Coverage 73.66% 73.66% -0.01%
==========================================
Files 368 368
Lines 36393 36395 +2
==========================================
+ Hits 26809 26810 +1
- Misses 9584 9585 +1 ☔ View full report in Codecov by Sentry. |
|
Hi Maria, it looks like the numpydoc-validation and ruff-format ignores lists were not updated to remove the exp_to_source check. Also, typically we are using Please review the instructions here: https://github.com/spacetelescope/jwst/wiki/Workflow#updating-a-module-for-code-style |
no needed changes for style changes
|
@emolter ready for another review, please. |
| @@ -1,5 +1,5 @@ | |||
| """exp_to_source: Reformat Level2b multi-source data to be source-based. | |||
| """ | |||
| """Step exp_to_source: Reformat Level2b multi-source data to be source-based.""" | |||
There was a problem hiding this comment.
| """Step exp_to_source: Reformat Level2b multi-source data to be source-based.""" | |
| """Reformat Level2b multi-source data to be source-based.""" |
| sources: {source: MultiExposureModel, ...} | ||
| A dict keyed on source name whose value is | ||
| the corresponding MultiExposureModel. | ||
| Docs from the source. |
There was a problem hiding this comment.
What does this mean?
I think this should say something to the effect of, "Command-line interface to the exp_to_source step. For help, use exp_to_source -h. See also the exp_to_source readthedocs page; https://link-to-page.html"
| """ | ||
|
|
||
| def __init__(self, args=None): | ||
| """ | ||
| Full documentation from the source. |
There was a problem hiding this comment.
| Full documentation from the source. | |
| Initialize and run the exp_to_source step. |
or something similar
| # sources : {source: MultiExposureModel, ...} | ||
| # A dict keyed on source name whose value is the corresponding MultiExposureModel. |
|
|
||
|
|
||
| INPUT_FILES_GLOB = 'data/jwst_nod?_cal.fits' | ||
| INPUT_FILES_GLOB = "data/" |
There was a problem hiding this comment.
with the change here, this is no longer really a "glob"; in fact, is this ever used anywhere now that the change is made? If so, could you please point me to where?
|
|
||
|
|
||
| # Calculate some extra constants | ||
| INPUT_FILES_GLOB = t_path(INPUT_FILES_GLOB) | ||
| INPUT_FILES = glob(t_path(INPUT_FILES_GLOB)) | ||
| INPUT_FILES = list(t_path("data").glob("jwst_nod*_cal.fits")) |
There was a problem hiding this comment.
Is there any reason this can no longer be jwst_nod?_cal.fits? It looks like the original intent was only to have a single-character wildcard, not a multi-character one.
There was a problem hiding this comment.
I doubt this is needed anymore, but it doesn't seem like a style change. Was this causing failed checks? Does anyone know the history of this?
| @@ -15,7 +15,7 @@ def test_default_run(tmp_path, capsys): | |||
| '-o', | |||
| str(tmp_path) | |||
| ] | |||
| args.extend(helpers.INPUT_FILES) | |||
| args.extend([str(infile) for infile in helpers.INPUT_FILES]) | |||
There was a problem hiding this comment.
why was this change necessary? (here and below). This looks like an indication that helpers.INPUT_FILES may have changed its behavior
Resolves partially JP-3858
Closes #
This PR addresses style changes to exp_to_source in the jwst repo.
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