Skip to content

Commit f90082b

Browse files
committed
address code-review findings on ngmix v2.0
- make_cat: read ngmix_mcal_flags/ngmix_id outside the moments branch; they were assigned only in the non-moments (else) branch but used unconditionally, so _save_ngmix_data(moments=True) raised NameError. - make_cat: remove a leftover debug print loop over the ELL columns. - runners (mccd_interp, read_ext_sexcat, psfex_interp, vignetmaker): the WCS log / exp-numbers files are positional inputs supplied via the config FILE_PATTERN in MULTI-EPOCH / post-process modes, but the @module_runner decorators only declare the base inputs (the optional ones cannot be encoded statically without breaking CLASSIC/base mode). Replace the bare input_file_list[1]/[2] reads with a clear ValueError naming the missing FILE_PATTERN entry instead of a cryptic IndexError. - ngmix compile_results: tie the hardcoded metacal-type list to METACAL_TYPES via a divergence check (order kept stable for byte-reproducible output). - ngmix: drop a stray marker comment and a dead commented-out vignet-path block; fold two debug prints into the ValueError message and fix a missing f-string prefix in an adjacent error message.
1 parent f0fca23 commit f90082b

6 files changed

Lines changed: 64 additions & 24 deletions

File tree

src/shapepipe/modules/make_cat_package/make_cat.py

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -356,6 +356,10 @@ def _save_ngmix_data(self, ngmix_cat_path, moments=False):
356356
#return err_msg
357357

358358
ngmix_mcal_types_fail = ngmix_cat_file.get_data()["mcal_types_fail"]
359+
# Needed in both moments and non-moments modes (used unconditionally
360+
# below), so read them outside the branch.
361+
ngmix_mcal_flags = ngmix_cat_file.get_data()["mcal_flags"]
362+
ngmix_id = ngmix_cat_file.get_data()["id"]
359363

360364
n_obj = len(self._obj_id)
361365
self._w_log.info(f"writing ngmix info for {n_obj} objects")
@@ -365,9 +369,6 @@ def _save_ngmix_data(self, ngmix_cat_path, moments=False):
365369
else:
366370
m = ""
367371

368-
ngmix_mcal_flags = ngmix_cat_file.get_data()["mcal_flags"]
369-
ngmix_id = ngmix_cat_file.get_data()["id"]
370-
371372
self._add2dict("NGMIX_N_EPOCH", np.zeros(n_obj))
372373
self._add2dict("NGMIX_MCAL_TYPES_FAIL", np.zeros(n_obj))
373374

@@ -397,12 +398,6 @@ def _save_ngmix_data(self, ngmix_cat_path, moments=False):
397398
)
398399
self._add2dict(f"NGMIX{m}_MCAL_FLAGS", np.zeros(len(self._obj_id)))
399400

400-
for idx, _ in enumerate(self._obj_id):
401-
for key in self._key_ends:
402-
x = self._output_dict[f"NGMIX{m}_ELL_{key}"][idx]
403-
if np.all(x != np.array([-10.0, -10.0])):
404-
print(x)
405-
406401
for idx, id_tmp in enumerate(self._obj_id):
407402
ind = np.where(id_tmp == ngmix_id)[0]
408403
if len(ind) > 0:

src/shapepipe/modules/mccd_interp_runner.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -91,6 +91,14 @@ def mccd_interp_runner(
9191
psf_model_dir = get_last_dir(run_dirs["run_log"], module)
9292
psf_model_pattern = config.get(module_config_sec, "PSF_MODEL_PATTERN")
9393
galcat_path = input_file_list[0]
94+
# The WCS log is supplied as a positional input (via FILE_PATTERN)
95+
# in MULTI-EPOCH mode, not by the decorator default.
96+
if len(input_file_list) < 2:
97+
raise ValueError(
98+
"MULTI-EPOCH mode requires the WCS log file as a second"
99+
+ " input; add 'log_exp_headers' to FILE_PATTERN and"
100+
+ f" FILE_EXT in the [{module_config_sec}] config section."
101+
)
94102
f_wcs_path = input_file_list[1]
95103

96104
inst = mccd_interp.MCCDinterpolator(

src/shapepipe/modules/ngmix_package/ngmix.py

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,6 @@ def get_prior(pixel_scale, rng, T_range=None, F_range=None):
8888
)
8989

9090

91-
# I still don't know how to handle this
9291
class Tile_cat():
9392
"""Tile_cat.
9493
@@ -282,13 +281,6 @@ def __init__(
282281
f_wcs_path,
283282
input_file_list[6] if len(input_file_list) == 7 else None,
284283
)
285-
#self._gal_vignet_path = input_file_list[1]
286-
#self._bkg_vignet_path = input_file_list[2]
287-
#self._psf_vignet_path = input_file_list[3]
288-
#self._weight_vignet_path = input_file_list[4]
289-
#self._flag_vignet_path = input_file_list[5]
290-
291-
292284

293285
self._output_dir = output_dir
294286
self._file_number_string = file_number_string
@@ -372,7 +364,15 @@ def compile_results(self, results):
372364
If SNR key not found
373365
374366
"""
367+
# Output HDU order. Same set as METACAL_TYPES, but kept in this
368+
# fixed order so output catalogues stay byte-reproducible; the check
369+
# below guards against the two lists silently diverging.
375370
names = ["1m", "1p", "2m", "2p", "noshear"]
371+
if set(names) != set(METACAL_TYPES):
372+
raise ValueError(
373+
"compile_results metacal type list is out of sync with"
374+
+ " METACAL_TYPES"
375+
)
376376
names2 = [
377377
'id',
378378
'n_epoch_model',
@@ -552,8 +552,8 @@ def save_results(self, output_dict):
552552
ext_name = hdu.name.lower()
553553
if ext_name not in output_dict:
554554
raise ValueError(
555-
"HDU extension {ext_name} from existing FITS file"
556-
+ f" not found in data"
555+
f"HDU extension {ext_name} from existing FITS"
556+
+ " file not found in data"
557557
)
558558

559559
# Existing data
@@ -569,10 +569,10 @@ def save_results(self, output_dict):
569569
colname in new_data
570570
for colname in existing_dtype.names
571571
):
572-
print("output_dict", [col for col in new_data])
573-
print("existing_da", existing_data.dtype.names)
574572
raise ValueError(
575-
"Mismatch between existing columns and new data columns."
573+
"Mismatch between existing columns"
574+
+ f" ({existing_dtype.names}) and new data"
575+
+ f" columns ({list(new_data)})."
576576
)
577577

578578
# New data to be appended

src/shapepipe/modules/psfex_interp_runner.py

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,6 +68,13 @@ def psfex_interp_runner(
6868
exp_base_dir = config.getexpanded(
6969
module_config_sec, "ME_DOT_PSF_EXP_DIR"
7070
)
71+
if len(input_file_list) < 3:
72+
raise ValueError(
73+
"ME_DOT_PSF_EXP_DIR requires the exposure-numbers file"
74+
+ " as a third input; add 'exp_numbers' to FILE_PATTERN"
75+
+ f" and FILE_EXT in the [{module_config_sec}] config"
76+
+ " section."
77+
)
7178
exp_numbers_file = input_file_list[2]
7279
dot_psf_dirs = get_exp_output_dirs(
7380
exp_base_dir, exp_numbers_file, "psfex_runner", w_log
@@ -92,8 +99,15 @@ def psfex_interp_runner(
9299
module_config_sec,
93100
"ME_DOT_PSF_PATTERN",
94101
)
95-
# Set input paths
102+
# Set input paths. The WCS log is a positional input (via
103+
# FILE_PATTERN) in MULTI-EPOCH mode, not a decorator default.
96104
galcat_path = input_file_list[0]
105+
if len(input_file_list) < 2:
106+
raise ValueError(
107+
"MULTI-EPOCH mode requires the WCS log file as a second"
108+
+ " input; add 'log_exp_headers' to FILE_PATTERN and"
109+
+ f" FILE_EXT in the [{module_config_sec}] config section."
110+
)
97111
f_wcs_path = input_file_list[1]
98112

99113
# Create instance of PSFExInterpolator

src/shapepipe/modules/read_ext_sexcat_runner.py

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,14 @@ def read_ext_sexcat_runner(
6161
)
6262

6363
if config.getboolean(module_config_sec, "MAKE_POST_PROCESS"):
64+
# The WCS log is supplied as a positional input (via FILE_PATTERN)
65+
# when post-processing is enabled, not by the decorator default.
66+
if len(input_file_list) < 3:
67+
raise ValueError(
68+
"MAKE_POST_PROCESS requires the WCS log file as a third"
69+
+ " input; add 'log_exp_headers' to FILE_PATTERN and"
70+
+ f" FILE_EXT in the [{module_config_sec}] config section."
71+
)
6472
f_wcs_path = input_file_list[2]
6573
pos_params = config.getlist(module_config_sec, "WORLD_POSITION")
6674
ccd_size = config.getlist(module_config_sec, "CCD_SIZE")

src/shapepipe/modules/vignetmaker_runner.py

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -110,6 +110,13 @@ def vignetmaker_runner(
110110
exp_base_dir = config.getexpanded(
111111
module_config_sec, "ME_IMAGE_EXP_DIR"
112112
)
113+
if len(input_file_list) < 3:
114+
raise ValueError(
115+
"ME_IMAGE_EXP_DIR requires the exposure-numbers"
116+
+ " file as a third input; add 'exp_numbers' to"
117+
+ " FILE_PATTERN and FILE_EXT in the"
118+
+ f" [{module_config_sec}] config section."
119+
)
113120
exp_numbers_file = input_file_list[2]
114121
exp_runner_names = config.getlist(
115122
module_config_sec, "ME_IMAGE_EXP_RUNNERS"
@@ -142,7 +149,15 @@ def vignetmaker_runner(
142149
"ME_IMAGE_PATTERN",
143150
)
144151

145-
# Get WCS log file path
152+
# Get WCS log file path. The WCS log is a positional input (via
153+
# FILE_PATTERN) in MULTI-EPOCH mode, not a decorator default.
154+
if len(input_file_list) < 2:
155+
raise ValueError(
156+
"MULTI-EPOCH mode requires the WCS log file as a second"
157+
+ " input; add 'log_exp_headers' to FILE_PATTERN and"
158+
+ f" FILE_EXT in the [{module_config_sec}] config"
159+
+ " section."
160+
)
146161
f_wcs_path = input_file_list[1]
147162

148163
# Process inputs

0 commit comments

Comments
 (0)