Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Collate1d bugfixes #1814

Merged
merged 4 commits into from
Jun 4, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 11 additions & 17 deletions pypeit/core/collate.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,23 +107,17 @@ def _config_key_match(self, header):
if 'PYP_SPEC' not in header or header['PYP_SPEC'] != self._spectrograph.name:
return False

first_header = self.spec1d_header_list[0]

for key in self._spectrograph.configuration_keys():
# Ignore "decker" because it's valid to coadd spectra with different slit masks
if key != "decker":
if key not in first_header and key not in header:
# Both are missing the key, this is ok
continue
elif key not in first_header or key not in header:
# One has a value and the other doesn't so they don't match
return False

if first_header[key] != header[key]:
return False

# No mismatches were found
return True
# Build the config to compare against from the first header,
# ignoring "decker" because it's valid to coadd spectra with different slit masks
first_config = {key: self.spec1d_header_list[0][key]
for key in self._spectrograph.configuration_keys() if key != 'decker'}

second_config = {key: header[key]
for key in self._spectrograph.configuration_keys() if key != 'decker'}

# Use spectrograph.same_configuration to compare the configurations, with check_keys set to False
# so only the keys in first_config are checked
return self._spectrograph.same_configuration([first_config, second_config],check_keys=False)

def match(self, spec_obj, spec1d_header, tolerance, unit = u.arcsec):
"""Determine if a SpecObj matches this group within the given tolerance.
Expand Down
8 changes: 5 additions & 3 deletions pypeit/par/parset.py
Original file line number Diff line number Diff line change
Expand Up @@ -372,9 +372,11 @@ def _data_string(data, use_repr=True, verbatim=False):
return '..' if len(data) == 0 else '``' + data + '``'
return data
if isinstance(data, list):
# TODO: When the list is empty, should the return include the
# brackets?
return '[]' if len(data) == 0 \
# When the list is empty, return an empty string, which config_lines will append a "," to.
# This allows ConfigObj to interpret it as an empty list, instead of string, when re-reading the
# configuration lines into a ParSet

return '' if len(data) == 0 \
else ', '.join([ ParSet._data_string(d, use_repr=use_repr,
verbatim=verbatim) for d in data ])
return data.__repr__() if use_repr else str(data)
Expand Down
6 changes: 3 additions & 3 deletions pypeit/par/pypeitpar.py
Original file line number Diff line number Diff line change
Expand Up @@ -5230,8 +5230,8 @@ def __init__(self, tolerance=None, dry_run=None, ignore_flux=None, flux=None, ma
descr = OrderedDict.fromkeys(pars.keys())

# Threshold for grouping by object
defaults['tolerance'] = '1.0'
dtypes['tolerance'] = [str, float]
defaults['tolerance'] = 1.0
dtypes['tolerance'] = [str, float, int]
descr['tolerance'] = "The tolerance used when comparing the coordinates of objects. If two " \
"objects are within this distance from each other, they " \
"are considered the same object. If match_using is 'ra/dec' (the default) " \
Expand Down Expand Up @@ -5269,7 +5269,7 @@ def __init__(self, tolerance=None, dry_run=None, ignore_flux=None, flux=None, ma

# What slit flags to exclude
defaults['exclude_slit_trace_bm'] = []
dtypes['exclude_slit_trace_bm'] = [list, str]
dtypes['exclude_slit_trace_bm'] = [list,str]
descr['exclude_slit_trace_bm'] = "A list of slit trace bitmask bits that should be excluded."

# What slit flags to exclude
Expand Down
163 changes: 100 additions & 63 deletions pypeit/tests/test_collate_1d.py
Original file line number Diff line number Diff line change
Expand Up @@ -52,42 +52,56 @@ def apply_helio(self, *args, **kwargs):


def mock_header(file):


if os.path.basename(file) == 'spec1d_file1':
return {'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DECKER': 'Z6CL01B',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'RA': '201.1517',
'DEC': '+27.3246',
'FILENAME': 'DE.20100913.22358',
'SEMESTER': '2019B',
'PROGID': 'TEST1'}
return fits.Header(
{'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'DECKER': 'Z6CL01B',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'RA': '201.1517',
'DEC': '+27.3246',
'FILENAME': 'DE.20100913.22358',
'SEMESTER': '2019B',
'PROGID': 'TEST1'})
elif os.path.basename(file) == 'spec1d_file3':
# Invalid header w/o MJD
return {'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DECKER': 'Z6CL01B',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'FILENAME': 'DE.20100913.22358',
'SEMESTER': '2019B',
'PROGID': 'TEST1'}
return fits.Header(
{'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'DECKER': 'Z6CL01B',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'FILENAME': 'DE.20100913.22358',
'SEMESTER': '2019B',
'PROGID': 'TEST1'})
else:
# Return a different decker to make sure it's properly ignored
return {'MJD': '58879.0',
'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DECKER': 'foo',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'RA': '201.0052',
'DEC': '+27.2418',
'FILENAME': 'DE.20100914.12358'}
return fits.Header(
{'MJD': '58879.0',
'PYP_SPEC': 'keck_deimos',
'DISPNAME': '830G',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'DECKER': 'foo',
'BINNING': '1,1',
'AIRMASS': '1.0',
'EXPTIME': '1200.0',
'RA': '201.0052',
'DEC': '+27.2418',
'FILENAME': 'DE.20100914.12358'})

class MockSpecObjs:
def __init__(self, file):
Expand Down Expand Up @@ -238,45 +252,68 @@ def test_config_key_match():

spectrograph = load_spectrograph('keck_deimos')

header1 = {'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'decker': 'Z6CL01B',
'binning': '1,1'}
header2 = {'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'decker': 'foo',
'binning': '1,1'}
header3 = {'dispname': '830L',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'decker': 'Z6CL01B',
'binning': '1,1'}
header4 = {'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'decker': 'Z6CL01B',
'binning': '1,1',
'amp': 'foo'}
header5 = {'dispname': '830G',
'MJD': '58878.0',
'decker': 'foo',
'binning': '1,1'}
header6 = {'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_nires',
'decker': 'foo',
'binning': '1,1'}
header1 = fits.Header(
{'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'decker': 'Z6CL01B',
'binning': '1,1'})
header2 = fits.Header(
{'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'decker': 'foo',
'binning': '1,1'})
header3 = fits.Header(
{'dispname': '830L',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'decker': 'Z6CL01B',
'binning': '1,1'})
header4 = fits.Header(
{'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_deimos',
'DISPANGLE': 8800.0,
'FILTER1': 'OG550',
'decker': 'Z6CL01B',
'binning': '1,1',
'amp': 'foo'})
header5 = fits.Header(
{'dispname': '830G',
'MJD': '58878.0',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'decker': 'foo',
'binning': '1,1'})

header6 = fits.Header(
{'dispname': '830G',
'MJD': '58878.0',
'PYP_SPEC': 'keck_nires',
'DISPANGLE': 8800.0,
'AMP': 'SINGLE:B',
'FILTER1': 'OG550',
'decker': 'foo',
'binning': '1,1'})

sobjs = mock_specobjs('spec1d_file1')
sobj = sobjs.specobjs[0]

so1 = SourceObject(sobj, header1, 'spec1d_file1', spectrograph, 'ra/dec')
so4 = SourceObject(sobj, header4, 'spec1d_file1', spectrograph, 'ra/dec')

# Test that config keys match regardless of decker and even if neither
# header has amp
# Test that config keys match regardless of decker
assert so1._config_key_match(header2) is True

# Test that config keys don't match if dispname doesn't match
Expand Down
63 changes: 63 additions & 0 deletions pypeit/tests/test_pypeitpar.py
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,69 @@ def test_readcfg():
pypeitpar.PypeItPar.from_cfg_file('default.cfg')
os.remove(default_file)

def print_diff_header(name1, name2):
print("----------------------------------------------------------------")
print(f"Comparing {name1} to {name2}")
print("----------------------------------------------------------------")

def diff_pars(par1, name1, par2, name2):
"""Print human readable diff output when comparing pars"""

printed_header = False
result = True
first_par_keys = set(par1.keys())
second_par_keys = set(par2.keys())
missing_from_second = first_par_keys - second_par_keys
if len(missing_from_second) > 0:
if not printed_header:
print_diff_header(name1, name2)
printed_header=True
print(f"{name1} keys missing from {name2}: {','.join(missing_from_second)}")
result=False

missing_from_first = second_par_keys - first_par_keys
if len(missing_from_first) > 0:
if not printed_header:
print_diff_header(name1, name2)
printed_header=True
print(f"{name2} keys missing from {name1}: {','.join(missing_from_second)}")
result=False

for key in first_par_keys & second_par_keys:
first_value = par1[key]
second_value = par2[key]
if isinstance(first_value, parset.ParSet) and isinstance(second_value, parset.ParSet):
if not diff_pars(first_value, f"{name1}->{key}", second_value, f"{name2}->{key}"):
result=False
else:
if not first_value == second_value:
result = False
if not printed_header:
print_diff_header(name1, name2)
printed_header=True
else:
print("\n")
print(f"{name1}->{key} differs from {name2}->{key}")
print(f"{name1}->{key} is type {type(first_value)}, value {first_value}")
print(f"{name2}->{key} is type {type(second_value)}, value {second_value}")

return result

def test_readwritecfg():
# Test writing and re-reading a file results in the same
# values
default_file = 'default.cfg'
if os.path.isfile(default_file):
os.remove(default_file)
default_par = pypeitpar.PypeItPar()
default_par.to_config(cfg_file=default_file)
assert os.path.isfile(default_file), 'No file written!'
read_par = pypeitpar.PypeItPar.from_cfg_file(default_file)

# Comparison that produces nice output when they differ
assert diff_pars(default_par, "Default", read_par, "Read")
os.remove(default_file)

def test_mergecfg():
# Create a file with the defaults
user_file = 'user_adjust.cfg'
Expand Down
Loading