From 9616df3a1d9c2c019e250b049a13ad3e795b471e Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Thu, 8 Aug 2024 11:54:44 -0400 Subject: [PATCH 1/9] Update log messages containing newlines --- jwst/ami/leastsqnrm.py | 6 ++++-- jwst/assign_wcs/assign_wcs_step.py | 6 ++++-- jwst/associations/association.py | 3 ++- jwst/dark_current/dark_current_step.py | 4 ++-- jwst/extract_1d/extract.py | 13 ++++++------- jwst/extract_1d/extract1d.py | 4 ++-- jwst/lib/set_telescope_pointing.py | 18 +++++++++--------- jwst/lib/tests/stp_v1.py | 24 +++++++++--------------- jwst/pipeline/calwebb_spec2.py | 14 ++++++++------ jwst/pixel_replace/pixel_replace_step.py | 4 ++-- jwst/scripts/adjust_wcs.py | 11 +++++------ jwst/scripts/set_telescope_pointing.py | 4 +++- jwst/scripts/v1_calculate.py | 11 +++++------ jwst/tweakreg/tweakreg_step.py | 8 ++++---- 14 files changed, 65 insertions(+), 65 deletions(-) diff --git a/jwst/ami/leastsqnrm.py b/jwst/ami/leastsqnrm.py index a798ada376..fbbcd98b5f 100644 --- a/jwst/ami/leastsqnrm.py +++ b/jwst/ami/leastsqnrm.py @@ -542,7 +542,8 @@ def matrix_operations(img, model, flux=None, linfit=False, dqm=None): flatimg = img.reshape(np.shape(img)[0] * np.shape(img)[1]) flatdqm = dqm.reshape(np.shape(img)[0] * np.shape(img)[1]) log.info("fringefitting.leastsqnrm.matrix_operations(): ") - log.info(f"\timg {img.shape:} \n\tdqm {dqm.shape:}") + log.info(f"\timg {img.shape:}") + log.info(f"\tdqm {dqm.shape:}") log.info( f"\tL x W = {img.shape[0]:d} x {img.shape[1]:d} = {img.shape[0] * img.shape[1]:d}", ) @@ -550,7 +551,8 @@ def matrix_operations(img, model, flux=None, linfit=False, dqm=None): log.info(f"\tflatdqm {flatdqm.shape:}") - log.info("\n\ttype(dqm) %s", type(dqm)) + log.info("") + log.info("\ttype(dqm) %s", type(dqm)) if dqm is not None: nanlist = np.where(flatdqm) # where DO_NOT_USE up. else: diff --git a/jwst/assign_wcs/assign_wcs_step.py b/jwst/assign_wcs/assign_wcs_step.py index 42b0818d03..1bfa023ac2 100755 --- a/jwst/assign_wcs/assign_wcs_step.py +++ b/jwst/assign_wcs/assign_wcs_step.py @@ -118,7 +118,8 @@ def process(self, input, *args, **kwargs): except (ValueError, RuntimeError) as e: log.warning("Failed to update 'meta.wcsinfo' with FITS SIP " - f'approximation. Reported error is:\n"{e.args[0]}"') + "approximation. Reported error is:") + log.warning(f'"{e.args[0]}"') else: # WFSS modes try: # A bounding_box is needed for the imaging WCS @@ -137,6 +138,7 @@ def process(self, input, *args, **kwargs): ) except (ValueError, RuntimeError) as e: log.warning("Failed to update 'meta.wcsinfo' with FITS SIP " - f'approximation. Reported error is:\n"{e.args[0]}"') + "approximation. Reported error is:") + log.warning(f'"{e.args[0]}"') return result diff --git a/jwst/associations/association.py b/jwst/associations/association.py index f5d681954d..1f91602ef3 100644 --- a/jwst/associations/association.py +++ b/jwst/associations/association.py @@ -198,7 +198,8 @@ def validate(cls, asn): try: jsonschema.validate(asn_data, asn_schema) except (AttributeError, jsonschema.ValidationError) as err: - logger.debug('Validation failed:\n%s', err) + logger.debug('Validation failed:') + logger.debug(str(err)) raise AssociationNotValidError('Validation failed') from err # Validate no path data for expnames diff --git a/jwst/dark_current/dark_current_step.py b/jwst/dark_current/dark_current_step.py index 2a0eddfc80..a7ae3c2761 100755 --- a/jwst/dark_current/dark_current_step.py +++ b/jwst/dark_current/dark_current_step.py @@ -100,8 +100,8 @@ def set_average_dark_current(self, input_model, dark_model): if np.sum(dark_model.average_dark_current) == 0.0: input_model.average_dark_current[:, :] = dark_model.meta.exposure.average_dark_current elif np.shape(input_model.average_dark_current) != np.shape(dark_model.average_dark_current): - self.log.warning("DarkModel average_dark_current does not match shape of data.\n" - "Dark current from reference file cannot be applied.") + self.log.warning("DarkModel average_dark_current does not match shape of data.") + self.log.warning("Dark current from reference file cannot be applied.") else: input_model.average_dark_current = dark_model.average_dark_current diff --git a/jwst/extract_1d/extract.py b/jwst/extract_1d/extract.py index 03b36b8968..7c0198bad7 100644 --- a/jwst/extract_1d/extract.py +++ b/jwst/extract_1d/extract.py @@ -805,13 +805,12 @@ def sanity_check_limits( """ if (ap_wcs.xstart >= ap_ref.xstop or ap_wcs.xstop <= ap_ref.xstart or ap_wcs.ystart >= ap_ref.ystop or ap_wcs.ystop <= ap_ref.ystart): - log.warning( - f"The WCS bounding box is outside the aperture:\n\t" - f"aperture: {ap_ref.xstart}, {ap_ref.xstop}, {ap_ref.ystart}, {ap_ref.ystop}\n\t" - f"wcs: {ap_wcs.xstart}, {ap_wcs.xstop}, {ap_wcs.ystart}, {ap_wcs.ystop}\n" - f"so the wcs bounding box will be ignored." - ) - + log.warning("The WCS bounding box is outside the aperture:") + log.warning(f"\taperture: {ap_ref.xstart}, {ap_ref.xstop}, " + f"{ap_ref.ystart}, {ap_ref.ystop}") + log.warning(f"\twcs: {ap_wcs.xstart}, {ap_wcs.xstop}, " + f"{ap_wcs.ystart}, {ap_wcs.ystop}") + log.warning("so the wcs bounding box will be ignored.") flag = False else: flag = True diff --git a/jwst/extract_1d/extract1d.py b/jwst/extract_1d/extract1d.py index e083497cd6..6f340b374b 100644 --- a/jwst/extract_1d/extract1d.py +++ b/jwst/extract_1d/extract1d.py @@ -269,8 +269,8 @@ def extract1d(image, var_poisson, var_rnoise, var_flat, lambdas, disp_range, elif len(bkg_model) < bkg_order: log.debug(f"Not enough valid pixels to determine background " f"with the required order for lambda={lam:.6f} " - f"(column {x:d})\n" - f"Lowering background order to {len(bkg_model)}") + f"(column {x:d})") + log.debug(f"Lowering background order to {len(bkg_model)}") # Extract the source, and optionally subtract background using the # fit to the background for this column. Even if diff --git a/jwst/lib/set_telescope_pointing.py b/jwst/lib/set_telescope_pointing.py index 111b415ce3..4f46a443ab 100644 --- a/jwst/lib/set_telescope_pointing.py +++ b/jwst/lib/set_telescope_pointing.py @@ -603,9 +603,9 @@ def add_wcs(filename, allow_any_file=False, force_level1bmodel=False, try: if type(model) not in EXPECTED_MODELS: - logger.warning(f'Input {model} is not of an expected type (uncal, rate, rateints)' - '\n Updating pointing may have no effect or detrimental effects on the WCS information,' - '\n especially if the input is the result of Level2b or higher calibration.') + logger.warning(f'Input {model} is not of an expected type (uncal, rate, rateints)') + logger.warning(' Updating pointing may have no effect or detrimental effects on the WCS information,') + logger.warning(' especially if the input is the result of Level2b or higher calibration.') if not allow_any_file: raise TypeError(f'Input model {model} is not one of {EXPECTED_MODELS} and `allow_any_file` is `False`.' '\n\tFailing WCS processing.') @@ -814,10 +814,10 @@ def update_wcs_from_fgs_guiding(model, t_pars, default_roll_ref=0.0, default_vpa model, t_pars, default_roll_ref, default_vparity, default_v3yangle ) - logger.info('WCS info:' - f'\n\tcrpix1: {crpix1} crpix2: {crpix2}' - f'\n\tcrval1: {crval1} crval2: {crval2}' - f'\n\tpc_matrix: {pc_matrix}') + logger.info('WCS info:') + logger.info(f'\tcrpix1: {crpix1} crpix2: {crpix2}') + logger.info(f'\tcrval1: {crval1} crval2: {crval2}') + logger.info(f'\tpc_matrix: {pc_matrix}') model.meta.wcsinfo.crpix1 = crpix1 model.meta.wcsinfo.crpix2 = crpix2 @@ -1242,8 +1242,8 @@ def calc_transforms_track_tr_202111(t_pars: TransformParameters): # Check on telemetry for FGS ID. If invalid, use either user-specified or default to 1. fgsid = t_pars.pointing.fgsid if fgsid not in FGSIDS: - logger.warning(f'Method {t_pars.method} requires a valid FGS ID in telementry.' - '\nHowever telemetry reports an invalid id of {fgsid}') + logger.warning(f'Method {t_pars.method} requires a valid FGS ID in telementry.') + logger.warning('However telemetry reports an invalid id of {fgsid}') if t_pars.fgsid in FGSIDS: fgsid = t_pars.fgsid logger.warning(f'Using user-specified ID of {fgsid}') diff --git a/jwst/lib/tests/stp_v1.py b/jwst/lib/tests/stp_v1.py index f6fcfb7b8b..625588398d 100644 --- a/jwst/lib/tests/stp_v1.py +++ b/jwst/lib/tests/stp_v1.py @@ -111,12 +111,10 @@ def add_wcs(filename): dec = pheader['TARG_DEC'] roll = 0 - logger.warning( - 'Cannot retrieve telescope pointing.' - '\n{}' - '\nUsing TARG_RA={}, TARG_DEC={} and PA_V3=0 ' - 'to set pointing.'.format(exception, ra, dec) - ) + logger.warning('Cannot retrieve telescope pointing.') + logger.warning(str(exception)) + logger.warning(f'Using TARG_RA={ra}, TARG_DEC={dec} and ' + f'PA_V3=0 to set pointing.') local_roll = compute_local_roll(roll, ra, dec, v2ref, v3ref) wcsinfo = (ra, dec, local_roll) @@ -136,10 +134,8 @@ def add_wcs(filename): v1_ra_deg, v1_dec_deg, v3_pa_deg = vinfo local_roll = compute_local_roll(v3_pa_deg, crval1, crval2, v2ref, v3ref) - logger.info( - 'Computed coordinates from quaternions:' - '\n\tRA = {} DEC={} PA_V3={}'.format(crval1, crval2, v3_pa_deg) - ) + logger.info('Computed coordinates from quaternions:') + logger.info(f'\tRA = {crval1} DEC={crval2} PA_V3={v3_pa_deg}') pheader['RA_V1'] = v1_ra_deg pheader['DEC_V1'] = v1_dec_deg @@ -339,11 +335,9 @@ def get_pointing(obsstart, obsend, result_type='first'): This will need be re-examined when more information is available. """ - logger.info( - 'Determining pointing between observations times (mjd):' - '\n\tobsstart = {}' - '\n\tobsend = {}'.format(obsstart, obsend) - ) + logger.info('Determining pointing between observations times (mjd):') + logger.info(f'\tobsstart = {obsstart}') + logger.info(f'\tobsend = {obsend}') try: engdb = ENGDB_Service() except Exception as exception: diff --git a/jwst/pipeline/calwebb_spec2.py b/jwst/pipeline/calwebb_spec2.py index d75f44dc72..37eafd0892 100644 --- a/jwst/pipeline/calwebb_spec2.py +++ b/jwst/pipeline/calwebb_spec2.py @@ -230,16 +230,18 @@ def process_exposure_product( assign_wcs_exception = exception if assign_wcs_exception is not None or \ calibrated.meta.cal_step.assign_wcs != 'COMPLETE': - message = ( - 'Assign_wcs processing was skipped.' - '\nAborting remaining processing for this exposure.' - '\nNo output product will be created.' + messages = ( + 'Assign_wcs processing was skipped.', + 'Aborting remaining processing for this exposure.', + 'No output product will be created.' ) if self.assign_wcs.skip: - self.log.warning(message) + for message in messages: + self.log.warning(message) return else: - self.log.error(message) + for message in messages: + self.log.error(message) if assign_wcs_exception is not None: raise assign_wcs_exception else: diff --git a/jwst/pixel_replace/pixel_replace_step.py b/jwst/pixel_replace/pixel_replace_step.py index 2c9b117e10..7aa07b3dfd 100644 --- a/jwst/pixel_replace/pixel_replace_step.py +++ b/jwst/pixel_replace/pixel_replace_step.py @@ -59,7 +59,7 @@ def process(self, input): self.log.debug('Input is a ModelContainer.') else: self.log.error(f'Input is of type {str(type(input_model))} for which') - self.log.error('pixel_replace does not have an algorithm.\n') + self.log.error('pixel_replace does not have an algorithm.') self.log.error('Pixel replacement will be skipped.') input_model.meta.cal_step.pixel_replace = 'SKIPPED' return input_model @@ -98,7 +98,7 @@ def process(self, input): self.log.debug('Input is a {model.meta.model_type}.') else: self.log.error(f'Input is of type {model.meta.model_type} for which') - self.log.error('pixel_replace does not have an algorithm.\n') + self.log.error('pixel_replace does not have an algorithm.') self.log.error('Pixel replacement will be skipped.') model.meta.cal_step.pixel_replace = 'SKIPPED' run_pixel_replace = False diff --git a/jwst/scripts/adjust_wcs.py b/jwst/scripts/adjust_wcs.py index d562745890..023d46971e 100755 --- a/jwst/scripts/adjust_wcs.py +++ b/jwst/scripts/adjust_wcs.py @@ -235,8 +235,8 @@ def main(): if wcs_pars == [0.0, 0.0, 0.0, 1.0]: logger.info( - "All WCS adjustment parameters ('ra_delta', 'dec_delta'," - "roll_delta, and scale_factor) have default values" + "All WCS adjustment parameters ('ra_delta', 'dec_delta', " + "'roll_delta', and 'scale_factor') have default values " "(identical transformation)." ) logger.info("Nothing to do. Quitting.") @@ -265,10 +265,9 @@ def main(): npoints=64 ) except (ValueError, RuntimeError) as e: - logger.warning( - "Failed to update 'meta.wcsinfo' with FITS SIP " - f'approximation. Reported error is:\n"{e.args[0]}"' - ) + logger.warning("Failed to update 'meta.wcsinfo' with " + "FITS SIP approximation. Reported error is:") + logger.warning(f'"{e.args[0]}"') if options.update: data_model.save(fname, overwrite=True) diff --git a/jwst/scripts/set_telescope_pointing.py b/jwst/scripts/set_telescope_pointing.py index 86f077e5e7..0b75ca0975 100755 --- a/jwst/scripts/set_telescope_pointing.py +++ b/jwst/scripts/set_telescope_pointing.py @@ -166,7 +166,9 @@ def main(): # Calculate WCS for all inputs. for filename in args.exposure: - logger.info('\n------' 'Setting pointing for {}'.format(filename)) + logger.info("") + logger.info('------') + logger.info('Setting pointing for {}'.format(filename)) # Create path for saving the transforms. transform_path = None diff --git a/jwst/scripts/v1_calculate.py b/jwst/scripts/v1_calculate.py index 91373a830c..567fa577fb 100755 --- a/jwst/scripts/v1_calculate.py +++ b/jwst/scripts/v1_calculate.py @@ -94,12 +94,11 @@ def main(): logger.info(f'Retrieving V1 over the time span {obsstart.isot} - {obsend.isot}') input_as_files = False if args.pointing != 'all': - logger.warning( - 'V1 pointings have been requested over a time range.' - ' However, the \'pointing\' option is not \'all\'.' - '\nThere will only be a single result returned. Is this what was desired?' - '\nSuggestion: Use \'--pointing=all\'' - ) + logger.warning("V1 pointings have been requested over a time range. " + "However, the 'pointing' option is not 'all'.") + logger.warning('There will only be a single result returned. ' + 'Is this what was desired?') + logger.warning("Suggestion: Use '--pointing=all'") else: input_as_files = True diff --git a/jwst/tweakreg/tweakreg_step.py b/jwst/tweakreg/tweakreg_step.py index 0ae6a51eb0..5255769fa2 100644 --- a/jwst/tweakreg/tweakreg_step.py +++ b/jwst/tweakreg/tweakreg_step.py @@ -356,9 +356,9 @@ def _apply_tweakreg_solution(self, crpix=None ) except (ValueError, RuntimeError) as e: - msg = f"Failed to update 'meta.wcsinfo' with FITS SIP \ - approximation. Reported error is: \n {e.args[0]}" - self.log.warning(msg) + self.log.warning("Failed to update 'meta.wcsinfo' with FITS SIP " + "approximation. Reported error is:") + self.log.warning(f'"{e.args[0]}"') record_step_status(image_model, "tweakreg", success=True) return image_model @@ -494,4 +494,4 @@ def _rename_catalog_columns(catalog): "columns 'x' and 'y' or 'xcentroid' and " "'ycentroid'." ) - return catalog \ No newline at end of file + return catalog From 2406bcf48c63b12d2dc4679c0f9940ab2af42db0 Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Thu, 8 Aug 2024 16:56:01 -0400 Subject: [PATCH 2/9] Add fixed slit name to level3 names if available --- jwst/associations/lib/dms_base.py | 6 ++---- jwst/associations/lib/rules_level3_base.py | 11 ++++++----- 2 files changed, 8 insertions(+), 9 deletions(-) diff --git a/jwst/associations/lib/dms_base.py b/jwst/associations/lib/dms_base.py index d3b885174f..97882a880f 100644 --- a/jwst/associations/lib/dms_base.py +++ b/jwst/associations/lib/dms_base.py @@ -765,7 +765,7 @@ def _get_slit_name(self): else: values.sort(key=str.lower) value = format_list(values) - if value not in _EMPTY: + if value not in _EMPTY and value not in slit_names: slit_names.append(value) # Build the string. Sort the elements in order to @@ -773,9 +773,7 @@ def _get_slit_name(self): slit_names.sort(key=str.lower) slit_name = '-'.join(slit_names) - if slit_name == '': - slit_name = None - + # Slit name may be empty string return slit_name def _get_subarray(self): diff --git a/jwst/associations/lib/rules_level3_base.py b/jwst/associations/lib/rules_level3_base.py index b16a444aed..f914c4596a 100644 --- a/jwst/associations/lib/rules_level3_base.py +++ b/jwst/associations/lib/rules_level3_base.py @@ -158,6 +158,10 @@ def _dms_product_name(association): opt_elem = association._get_opt_element() + slit_name = association._get_slit_name() + if slit_name: + slit_name = '-' + slit_name + exposure = association._get_exposure() if exposure: exposure = '-' + exposure @@ -170,7 +174,7 @@ def _dms_product_name(association): 'jw{program}-{acid}' '_{target}' '_{instrument}' - '_{opt_elem}{subarray}' + '_{opt_elem}{slit_name}{subarray}' ) product_name = product_name.format( program=association.data['program'], @@ -178,6 +182,7 @@ def _dms_product_name(association): target=target, instrument=instrument, opt_elem=opt_elem, + slit_name=slit_name, subarray=subarray, exposure=exposure ) @@ -674,10 +679,6 @@ def dms_product_name_nrsfs_sources(asn): opt_elem = asn._get_opt_element() - slit_name = asn._get_slit_name() - if slit_name: - slit_name = '-' + slit_name - subarray = asn._get_subarray() if subarray: subarray = '-' + subarray From da148e64b3bbb46788379e2bbcc39f03c9fd2262 Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Thu, 8 Aug 2024 18:29:31 -0400 Subject: [PATCH 3/9] More specific tests for level3 product names --- jwst/associations/lib/rules_level3_base.py | 7 +++- .../tests/test_level3_product_names.py | 32 ++++++++++++++----- 2 files changed, 30 insertions(+), 9 deletions(-) diff --git a/jwst/associations/lib/rules_level3_base.py b/jwst/associations/lib/rules_level3_base.py index f914c4596a..5cec88f0e6 100644 --- a/jwst/associations/lib/rules_level3_base.py +++ b/jwst/associations/lib/rules_level3_base.py @@ -638,6 +638,10 @@ def dms_product_name_sources(asn): opt_elem = asn._get_opt_element() + slit_name = asn._get_slit_name() + if slit_name: + slit_name = '-' + slit_name + subarray = asn._get_subarray() if subarray: subarray = '-' + subarray @@ -646,7 +650,7 @@ def dms_product_name_sources(asn): 'jw{program}-{acid}' '_{source_id}' '_{instrument}' - '_{opt_elem}{subarray}' + '_{opt_elem}{slit_name}{subarray}' ) product_name = format_product( product_name_format, @@ -654,6 +658,7 @@ def dms_product_name_sources(asn): acid=asn.acid.id, instrument=instrument, opt_elem=opt_elem, + slit_name=slit_name, subarray=subarray, ) diff --git a/jwst/associations/tests/test_level3_product_names.py b/jwst/associations/tests/test_level3_product_names.py index 5259bf2a76..88b2d65f94 100644 --- a/jwst/associations/tests/test_level3_product_names.py +++ b/jwst/associations/tests/test_level3_product_names.py @@ -117,11 +117,27 @@ def test_multiple_optelems(pool_file): if asn['asn_rule'] != 'Asn_Lv3MIRMRS': m = re.match(LEVEL3_PRODUCT_NAME_REGEX, product_name) assert m is not None - try: - value = '-'.join(asn.constraints['opt_elem2'].found_values) - except KeyError: - value = None - if value in EMPTY: - assert '-' not in m.groupdict()['opt_elem'] - else: - assert '-' in m.groupdict()['opt_elem'] + + # should always be an opt_elem + values = ['-'.join(asn.constraints['opt_elem'].found_values)] + + # may be an opt_elem2, fixed slit or 2, or a subarray + for extra in ['opt_elem2', 'fxd_slit', 'fxd_slit2', 'subarray']: + + # special rules for fixed slit for NRS FS: + # it gets a format placeholder instead of the value + if asn['asn_rule'] == 'Asn_Lv3NRSFSS': + if extra == 'fxd_slit': + values.append('{slit_name}') + continue + elif extra == 'fxd_slit2': + continue + + try: + value = '-'.join(asn.constraints[extra].found_values) + except KeyError: + value = '' + if value: + values.append(value) + + assert m.groupdict()['opt_elem'] == '-'.join(values) From f28d8f8d6f629757c608b9951e49f05aab63dcdf Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Fri, 9 Aug 2024 09:45:58 -0400 Subject: [PATCH 4/9] Update comments for clarity --- jwst/associations/lib/rules_level3_base.py | 6 ++++-- jwst/associations/tests/test_level3_product_names.py | 8 ++++---- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/jwst/associations/lib/rules_level3_base.py b/jwst/associations/lib/rules_level3_base.py index 5cec88f0e6..b072f6cbea 100644 --- a/jwst/associations/lib/rules_level3_base.py +++ b/jwst/associations/lib/rules_level3_base.py @@ -666,8 +666,10 @@ def dms_product_name_sources(asn): def dms_product_name_nrsfs_sources(asn): - """Produce source-based product names for - NIRSpec fixed-slit observations. + """Produce source-based product names for NIRSpec fixed-slit observations. + + For this mode, the product names have a placeholder for the + slit name, to be filled in later by the pipeline. Parameters --------- diff --git a/jwst/associations/tests/test_level3_product_names.py b/jwst/associations/tests/test_level3_product_names.py index 88b2d65f94..1da85ce9a6 100644 --- a/jwst/associations/tests/test_level3_product_names.py +++ b/jwst/associations/tests/test_level3_product_names.py @@ -118,10 +118,10 @@ def test_multiple_optelems(pool_file): m = re.match(LEVEL3_PRODUCT_NAME_REGEX, product_name) assert m is not None - # should always be an opt_elem + # there should always be an opt_elem values = ['-'.join(asn.constraints['opt_elem'].found_values)] - # may be an opt_elem2, fixed slit or 2, or a subarray + # there may also be an opt_elem2, fixed slit or 2, or a subarray for extra in ['opt_elem2', 'fxd_slit', 'fxd_slit2', 'subarray']: # special rules for fixed slit for NRS FS: @@ -136,8 +136,8 @@ def test_multiple_optelems(pool_file): try: value = '-'.join(asn.constraints[extra].found_values) except KeyError: - value = '' - if value: + value = None + if value not in EMPTY: values.append(value) assert m.groupdict()['opt_elem'] == '-'.join(values) From b4a4ccf23f200b0ddf9e6a095706da3fb08fac75 Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Fri, 9 Aug 2024 11:09:08 -0400 Subject: [PATCH 5/9] Update tests to check more conditions --- .../tests/test_level3_product_names.py | 38 +++++++++++++++++-- 1 file changed, 34 insertions(+), 4 deletions(-) diff --git a/jwst/associations/tests/test_level3_product_names.py b/jwst/associations/tests/test_level3_product_names.py index 1da85ce9a6..74213b024b 100644 --- a/jwst/associations/tests/test_level3_product_names.py +++ b/jwst/associations/tests/test_level3_product_names.py @@ -96,7 +96,7 @@ def test_level3_names(pool_file, global_constraints): rules = registry_level3_only( global_constraints=global_constraints ) - pool = AssociationPool.read(pool_file) + pool = combine_pools(pool_file) asns = generate(pool, rules) for asn in asns: product_name = asn['products'][0]['name'] @@ -110,8 +110,8 @@ def test_level3_names(pool_file, global_constraints): def test_multiple_optelems(pool_file): rules = registry_level3_only() - pool = AssociationPool.read(pool_file) - asns = generate(pool, rules) + pool = combine_pools(pool_file) + asns = generate(pool, rules, finalize=False) for asn in asns: product_name = asn['products'][0]['name'] if asn['asn_rule'] != 'Asn_Lv3MIRMRS': @@ -137,7 +137,37 @@ def test_multiple_optelems(pool_file): value = '-'.join(asn.constraints[extra].found_values) except KeyError: value = None - if value not in EMPTY: + + # empty values and subarray = full are not recorded + if value not in EMPTY and value != 'full': values.append(value) assert m.groupdict()['opt_elem'] == '-'.join(values) + + +def test_tso3_names(): + rules = registry_level3_only() + tso_pool = t_path('data/pool_021_tso.csv') + pool = combine_pools(tso_pool) + asns = generate(pool, rules, finalize=False) + for asn in asns: + product_name = asn['products'][0]['name'] + + m = re.match(LEVEL3_PRODUCT_NAME_REGEX, product_name) + assert m is not None + + # there should always be an opt_elem + values = ['-'.join(asn.constraints['opt_elem'].found_values)] + + # there may also be an opt_elem2, fixed slit or 2, or a subarray + for extra in ['opt_elem2', 'fxd_slit', 'fxd_slit2', 'subarray']: + try: + value = '-'.join(asn.constraints[extra].found_values) + except KeyError: + value = None + + # empty values and subarray = full are not recorded + if value not in EMPTY and value != 'full': + values.append(value) + + assert m.groupdict()['opt_elem'] == '-'.join(values) From 253e46eaa726bf1b2caa42c044ed6bd50002323d Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Fri, 9 Aug 2024 12:02:39 -0400 Subject: [PATCH 6/9] Update change log --- CHANGES.rst | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGES.rst b/CHANGES.rst index 23ed8e6807..bac9999306 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -17,6 +17,12 @@ assign_wcs - Moved `update_s_region_imaging`, `update_s_region_keyword`, and `wcs_from_footprints` into stcal. [#8624] +associations +------------ + +- Restored slit name to level 3 product names for NIRSpec BOTS and background + fixed slit targets. [#8699] + cube_build ---------- From aeb7469b2cce0d1dc97a2f44c1791592b9a66fc4 Mon Sep 17 00:00:00 2001 From: Melanie Clarke Date: Fri, 9 Aug 2024 12:13:55 -0400 Subject: [PATCH 7/9] Code style fix --- jwst/associations/tests/test_level3_product_names.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/jwst/associations/tests/test_level3_product_names.py b/jwst/associations/tests/test_level3_product_names.py index 74213b024b..c77a146201 100644 --- a/jwst/associations/tests/test_level3_product_names.py +++ b/jwst/associations/tests/test_level3_product_names.py @@ -9,7 +9,7 @@ t_path, ) -from jwst.associations import (AssociationPool, generate) +from jwst.associations import generate from jwst.associations.lib.dms_base import DMSAttrConstraint From 8efc26fdbda7936199d1a05cb57707d2c1cb0665 Mon Sep 17 00:00:00 2001 From: Brett Graham Date: Tue, 20 Aug 2024 15:38:39 -0400 Subject: [PATCH 8/9] bump stpipe (#8713) --- CHANGES.rst | 2 ++ pyproject.toml | 2 +- 2 files changed, 3 insertions(+), 1 deletion(-) diff --git a/CHANGES.rst b/CHANGES.rst index b973b457be..d507be5fe6 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -36,6 +36,8 @@ general - Update required stcal version to 1.8.0. [#8706] +- Increase minimum required stpipe. [#8713] + master_background ----------------- diff --git a/pyproject.toml b/pyproject.toml index 8b00b35583..ff6b557472 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,7 +36,7 @@ dependencies = [ "spherical-geometry>=1.2.22", "stcal>=1.8.0,<1.9.0", "stdatamodels>=2.0.0,<2.1.0", - "stpipe>=0.6.0,<0.7.0", + "stpipe>=0.7.0,<0.8.0", "stsci.imagestats>=1.6.3", "synphot>=1.2", "tweakwcs>=0.8.8", From 3c053d10a1d4a04dbd642bbd61d17c706974cc43 Mon Sep 17 00:00:00 2001 From: Zhen-Kai Gao Date: Tue, 27 Aug 2024 00:50:03 +0800 Subject: [PATCH 9/9] Fix a typo in `load_custom_wcs` (#8698) Co-authored-by: Melanie Clarke --- CHANGES.rst | 7 + jwst/resample/resample_spec.py | 4 +- jwst/resample/resample_step.py | 16 +- jwst/resample/tests/test_resample_step.py | 214 +++++++++++++++++++++- 4 files changed, 232 insertions(+), 9 deletions(-) diff --git a/CHANGES.rst b/CHANGES.rst index d507be5fe6..47b2856642 100644 --- a/CHANGES.rst +++ b/CHANGES.rst @@ -79,6 +79,13 @@ resample_spec so that the spectral resampling step only exposes parameters that are appropriate for spectral data. [#8622] +resample_step +------------- + +- Fixed a typo in ``load_custom_wcs`` from ``array_shape`` to ``pixel_shape`` and + changed to use values in the top-level ASDF structure if the values in the WCS + are ``None``. [#8698] + scripts ------- diff --git a/jwst/resample/resample_spec.py b/jwst/resample/resample_spec.py index f3f6414011..54791bfa5d 100644 --- a/jwst/resample/resample_spec.py +++ b/jwst/resample/resample_spec.py @@ -126,9 +126,7 @@ def __init__(self, input_models, output=None, single=False, blendheaders=False, log.warning("Unable to compute output pixel area " "from 'output_wcs'.") output_pix_area = None - else: # pragma: no cover - # This clause is not reachable under usual circumstances: - # gwcs WCS discards the pixel_area attribute when saved as ASDF + else: log.debug(f'Setting output pixel area from wcs.pixel_area: ' f'{output_wcs.pixel_area}') output_pix_area = output_wcs.pixel_area diff --git a/jwst/resample/resample_step.py b/jwst/resample/resample_step.py index 444a2a9050..4c2d647985 100755 --- a/jwst/resample/resample_step.py +++ b/jwst/resample/resample_step.py @@ -176,9 +176,16 @@ def load_custom_wcs(asdf_wcs_file, output_shape=None): with asdf.open(asdf_wcs_file) as af: wcs = deepcopy(af.tree["wcs"]) - wcs.pixel_area = af.tree.get("pixel_area", None) - wcs.array_shape = af.tree.get("pixel_shape", None) - wcs.array_shape = af.tree.get("array_shape", None) + pixel_area = af.tree.get("pixel_area", None) + pixel_shape = af.tree.get("pixel_shape", None) + array_shape = af.tree.get("array_shape", None) + + if not hasattr(wcs, "pixel_area") or wcs.pixel_area is None: + wcs.pixel_area = pixel_area + if not hasattr(wcs, "pixel_shape") or wcs.pixel_shape is None: + wcs.pixel_shape = pixel_shape + if not hasattr(wcs, "array_shape") or wcs.array_shape is None: + wcs.array_shape = array_shape if output_shape is not None: wcs.array_shape = output_shape[::-1] @@ -192,10 +199,11 @@ def load_custom_wcs(asdf_wcs_file, output_shape=None): int(axs[1] + 0.5) for axs in wcs.bounding_box.bounding_box(order="C") ) + wcs.pixel_shape = wcs.array_shape[::-1] else: raise ValueError( "Step argument 'output_shape' is required when custom WCS " - "does not have neither of 'array_shape', 'pixel_shape', or " + "does not have 'array_shape', 'pixel_shape', or " "'bounding_box' attributes set." ) diff --git a/jwst/resample/tests/test_resample_step.py b/jwst/resample/tests/test_resample_step.py index 7f8c479f99..199000c1f9 100644 --- a/jwst/resample/tests/test_resample_step.py +++ b/jwst/resample/tests/test_resample_step.py @@ -832,7 +832,7 @@ def test_custom_refwcs_resample_imaging(nircam_rate, output_shape2, match, refwcs = str(tmp_path / "resample_refwcs.asdf") result.meta.wcs.bounding_box = [(-0.5, 1204.5), (-0.5, 1099.5)] - asdf.AsdfFile({"wcs": result.meta.wcs}).write_to(tmp_path / refwcs) + asdf.AsdfFile({"wcs": result.meta.wcs}).write_to(refwcs) result = ResampleStep.call( im, @@ -865,6 +865,66 @@ def test_custom_refwcs_resample_imaging(nircam_rate, output_shape2, match, result.close() +def test_custom_refwcs_pixel_shape_imaging(nircam_rate, tmp_path): + + # make some data with a WCS and some random values + im = AssignWcsStep.call(nircam_rate, sip_approx=False) + rng = np.random.default_rng(seed=77) + im.data[:, :] = rng.random(im.data.shape) + + crpix = (600, 550) + crval = (22.04, 11.98) + rotation = 15 + ratio = 0.7 + + # first pass - create a reference output WCS: + result = ResampleStep.call( + im, + output_shape=(1205, 1100), + crpix=crpix, + crval=crval, + rotation=rotation, + pixel_scale_ratio=ratio + ) + + # make sure results are nontrivial + data1 = result.data + assert not np.all(np.isnan(data1)) + + # remove the bounding box so shape is set from pixel_shape + # and also set a top-level pixel area + pixel_area = 1e-13 + refwcs = str(tmp_path / "resample_refwcs.asdf") + result.meta.wcs.bounding_box = None + asdf.AsdfFile({"wcs": result.meta.wcs, + "pixel_area": pixel_area}).write_to(refwcs) + + result = ResampleStep.call(im, output_wcs=refwcs) + + data2 = result.data + assert not np.all(np.isnan(data2)) + + # test output image shape + assert data1.shape == data2.shape + assert np.allclose(data1, data2, equal_nan=True) + + # make sure pixel values are similar, accounting for scale factor + # (assuming inputs are in surface brightness units) + iscale = np.sqrt(im.meta.photometry.pixelarea_steradians + / compute_image_pixel_area(im.meta.wcs)) + input_mean = np.nanmean(im.data) + output_mean_1 = np.nanmean(data1) + output_mean_2 = np.nanmean(data2) + assert np.isclose(input_mean * iscale**2, output_mean_1, atol=1e-4) + assert np.isclose(input_mean * iscale**2, output_mean_2, atol=1e-4) + + # check that output pixel area is set from input + assert np.isclose(result.meta.photometry.pixelarea_steradians, pixel_area) + + im.close() + result.close() + + @pytest.mark.parametrize('ratio', [0.7, 1.0, 1.3]) def test_custom_refwcs_resample_miri(miri_cal, tmp_path, ratio): im = miri_cal @@ -936,7 +996,7 @@ def test_custom_refwcs_resample_nirspec(nirspec_cal, tmp_path, ratio): # save the wcs from the output refwcs = str(tmp_path / "resample_refwcs.asdf") - asdf.AsdfFile({"wcs": result.slits[0].meta.wcs}).write_to(tmp_path / refwcs) + asdf.AsdfFile({"wcs": result.slits[0].meta.wcs}).write_to(refwcs) # run again, this time using the created WCS as input result = ResampleSpecStep.call(im, output_wcs=refwcs) @@ -961,6 +1021,52 @@ def test_custom_refwcs_resample_nirspec(nirspec_cal, tmp_path, ratio): result.close() +def test_custom_refwcs_pixel_shape_nirspec(nirspec_cal, tmp_path): + im = nirspec_cal + for slit in im.slits: + slit.meta.bunit_data = "MJy/sr" + + # mock a spectrum by giving the first slit some random + # values at the center + rng = np.random.default_rng(seed=77) + new_values = rng.random(im.slits[0].data.shape) + + center = im.slits[0].data.shape[0] // 2 + im.slits[0].data[:] = 0.0 + im.slits[0].data[center - 2:center + 2, :] = new_values[center - 2:center + 2, :] + + # first pass: create a reference output WCS with a custom pixel scale + ratio = 0.7 + result = ResampleSpecStep.call(im, pixel_scale_ratio=ratio) + + # make sure results are nontrivial + data1 = result.slits[0].data + assert not np.all(np.isnan(data1)) + + # remove the bounding box from the WCS so shape is set from pixel_shape + # and also set a top-level pixel area + pixel_area = 1e-13 + refwcs = str(tmp_path / "resample_refwcs.asdf") + asdf.AsdfFile({"wcs": result.slits[0].meta.wcs, + "pixel_area": pixel_area}).write_to(refwcs) + + # run again, this time using the created WCS as input + result = ResampleSpecStep.call(im, output_wcs=refwcs) + + data2 = result.slits[0].data + assert not np.all(np.isnan(data2)) + + # check output data against first pass + assert data1.shape == data2.shape + assert np.allclose(data1, data2, equal_nan=True, rtol=1e-4) + + # check that output pixel area is set from output_wcs + assert np.isclose(result.slits[0].meta.photometry.pixelarea_steradians, pixel_area) + + im.close() + result.close() + + @pytest.mark.parametrize('ratio', [1.3, 1]) def test_custom_wcs_pscale_resample_imaging(nircam_rate, ratio): im = AssignWcsStep.call(nircam_rate, sip_approx=False) @@ -1000,6 +1106,7 @@ def test_custom_wcs_pscale_resample_miri(miri_cal, ratio): result.close() + @pytest.mark.parametrize('ratio', [1.3, 1]) def test_custom_wcs_pscale_resample_nirspec(nirspec_cal, ratio): im = nirspec_cal.slits[0] @@ -1019,6 +1126,109 @@ def test_custom_wcs_pscale_resample_nirspec(nirspec_cal, ratio): result.close() +@pytest.mark.parametrize('wcs_attr', ['pixel_shape', 'array_shape', 'bounding_box']) +def test_custom_wcs_input(tmp_path, nircam_rate, wcs_attr): + # make a valid WCS + im = AssignWcsStep.call(nircam_rate, sip_approx=False) + wcs = im.meta.wcs + + # store values in a dictionary + wcs_dict = {'array_shape': im.data.shape, + 'pixel_shape': im.data.shape[::-1], + 'bounding_box': wcs.bounding_box} + + # Set all attributes to None + for attr in ['pixel_shape', 'array_shape', 'bounding_box']: + setattr(wcs, attr, None) + + # Set the attribute to the correct value + setattr(wcs, wcs_attr, wcs_dict[wcs_attr]) + + # write the WCS to an asdf file + refwcs = str(tmp_path / 'test_wcs.asdf') + asdf.AsdfFile({"wcs": wcs}).write_to(refwcs) + + # load the WCS from the asdf file + loaded_wcs = ResampleStep.load_custom_wcs(refwcs) + + # check that the loaded WCS has the correct values + for attr in ['pixel_shape', 'array_shape']: + assert np.allclose(getattr(loaded_wcs, attr), wcs_dict[attr]) + + +@pytest.mark.parametrize('override,value', + [('pixel_area', 1e-13), ('pixel_shape', (300, 400)), + ('array_shape', (400, 300))]) +def test_custom_wcs_input_overrides(tmp_path, nircam_rate, override, value): + # make a valid WCS + im = AssignWcsStep.call(nircam_rate, sip_approx=False) + wcs = im.meta.wcs + + # remove existing shape keys if testing shape overrides + if override != 'pixel_area': + wcs.pixel_shape = None + wcs.bounding_box = None + + expected_array_shape = im.data.shape + expected_pixel_shape = im.data.shape[::-1] + expected_pixel_area = None + + # write the WCS to an asdf file with a top-level override + refwcs = str(tmp_path / 'test_wcs.asdf') + asdf.AsdfFile({"wcs": wcs, override: value}).write_to(refwcs) + + # check for expected values when read back in + keys = ['pixel_area', 'pixel_shape', 'array_shape'] + loaded_wcs = ResampleStep.load_custom_wcs(refwcs) + for key in keys: + if key == override: + assert np.allclose(getattr(loaded_wcs, key), value) + elif key == 'pixel_shape': + if override == 'array_shape': + assert np.allclose(getattr(loaded_wcs, key), value[::-1]) + else: + assert np.allclose(getattr(loaded_wcs, key), expected_pixel_shape) + elif key == 'array_shape': + if override == 'pixel_shape': + assert np.allclose(getattr(loaded_wcs, key), value[::-1]) + else: + assert np.allclose(getattr(loaded_wcs, key), expected_array_shape) + elif key == 'pixel_area': + assert getattr(loaded_wcs, key) == expected_pixel_area + + +def test_custom_wcs_input_error(tmp_path, nircam_rate): + # make a valid WCS + im = AssignWcsStep.call(nircam_rate, sip_approx=False) + wcs = im.meta.wcs + + # remove shape settings + wcs.pixel_shape = None + wcs.array_shape = None + wcs.bounding_box = None + + # write the WCS to an asdf file + refwcs = str(tmp_path / 'test_wcs.asdf') + asdf.AsdfFile({"wcs": wcs}).write_to(refwcs) + + # loading the file without shape info should produce an error + with pytest.raises(ValueError, match="'output_shape' is required"): + loaded_wcs = ResampleStep.load_custom_wcs(refwcs) + + # providing an output shape should succeed + output_shape = (300, 400) + loaded_wcs = ResampleStep.load_custom_wcs(refwcs, output_shape=output_shape) + + # array shape is opposite of input values (numpy convention) + assert np.all(loaded_wcs.array_shape == output_shape[::-1]) + + # pixel shape matches + assert np.all(loaded_wcs.pixel_shape == output_shape) + + # bounding box is not set + assert loaded_wcs.bounding_box is None + + def test_pixscale(nircam_rate): # check that if both 'pixel_scale_ratio' and 'pixel_scale' are passed in,