Merge remote-tracking branch 'origin/develop' into hires_waves_and_re…

…factor

CHANGES.rst

@@ -1,5 +1,5 @@
-1.13.1dev (6 June 2023)
-------------------------
+1.13.1dev
+---------
 
 - Add support for Gemini/GNIRS (IFU)
 - Added a script to convert a wavelength solution into something that can be placed in the reid archive.
@@ -33,6 +33,10 @@
     - Now ``only_slits`` parameter in `pypeit_coadd_2dspec` includes the detector number (similar to ``slitspatnum``)
     - Added ``exclude_slits`` parameter in `pypeit_coadd_2dspec` to exclude specific slits
     - Fix wrong RA & Dec for 2D coadded serendips
+- Changed calibration frame naming as an attempt to avoid very long names for
+  files with many calibration groups.  Sequential numbers are reduced to a
+  range; e.g., ``'0-1-2-3-4'`` becomes ``'0+4'`` and
+  ``'3-5-6-10-11-12-15-18-19'`` becomes ``'3-5+6-10+12-15-18+19'``
 - HIRES wavelength solution improvements galor
 - Added `redo_slits` option
 - Refactored ``load_line_lists()`` yet again!

doc/calibrations/calibrations.rst

@@ -175,15 +175,22 @@ All reduced calibration frames are named according to their primary calibration
 type (e.g., ``Arc``).  They are also assigned a unique identifier that is a
 combination of:
 
-    - the instrument configuration (setup) identifier (e.g., ``A``),
+    #. the instrument configuration (setup) identifier (e.g., ``A``),
 
-    - the list of associated calibration groups (e.g., ``1-2`` or ``all``), and
+    #. a compressed list of associated calibration groups (e.g., ``1+2`` or ``all``), and
 
-    - the detector or mosaic identifier (e.g., ``DET01`` or ``MSC01``).
+    #. the detector or mosaic identifier (e.g., ``DET01`` or ``MSC01``).
 
-.. note::
+For the second component, sequential numbers are reduced to a range; e.g.,
+``'0-1-2-3-4'`` becomes ``'0+4'`` and ``'3-5-6-10-11-12-15-18-19'`` becomes
+``'3-5+6-10+12-15-18+19'``.
+
+.. warning::
 
     If you have a lot of calibration groups in your pypeit file, you may end up
-    with very long file names!
+    with very long file names!  This may cause a fault when the file name is
+    included in the header of the output fits files.  If using the calibration
+    group ``all`` doesn't solve the problem or isn't possible given your
+    application, please `Submit an issue`_.
 
 

doc/scripts/make_example_files.py

@@ -75,10 +75,11 @@ def make_example_gnirs_pypeit_files(version, date):
     oroot = Path(resource_filename('pypeit', '')).resolve().parent / 'doc' / 'include'
 
     # Create the default pypeit file
-    droot = Path(os.getenv('PYPEIT_DEV')).resolve() / 'RAW_DATA' / 'gemini_gnirs_echelle' / '32_SB_SXD'
+    droot = Path(os.getenv('PYPEIT_DEV')).resolve() / 'RAW_DATA' / 'gemini_gnirs_echelle' \
+                / '32_SB_SXD'
 
-    pargs = setup.Setup.parse_args(['-r', str(droot), '-s', 'gemini_gnirs_echelle', '-b', '-c', 'A',
-                                    '-d', str(oroot),
+    pargs = setup.Setup.parse_args(['-r', str(droot), '-s', 'gemini_gnirs_echelle', '-b',
+                                    '-c', 'A', '-d', str(oroot),
                                     '--version_override', version, 
                                     '--date_override', date])
     setup.Setup.main(pargs)

pypeit/calibframe.py

@@ -302,6 +302,74 @@ def ingest_calib_id(calib_id):
                 msgs.error(f'Invalid calibration group {c}; must be convertible to an integer.')
         return _calib_id.tolist()
 
+    @staticmethod
+    def construct_calib_id(calib_id, ingested=False):
+        """
+        Use the calibration ID to construct a unique identifying string included
+        in output file names.
+
+        Args:
+            calib_id (:obj:`str`, :obj:`list`, :obj:`int`):
+                Identifiers for one or more calibration groups for this
+                calibration frame.  Strings (either as individually entered or
+                as elements of a provided list) can be single or comma-separated
+                integers.  Otherwise, all strings must be convertible to
+                integers; the only exception is the string 'all'.
+            ingested (:obj:`bool`, optional):
+                Indicates that the ``calib_id`` object has already been
+                "ingested" (see :func:`ingest_calib_id`).  If True, this will
+                skip the ingestion step.
+
+        Returns:
+            :obj:`str`: A string identifier to include in output file names.
+        """
+        # Ingest the calibration IDs, if necessary
+        _calib_id = calib_id if ingested else CalibFrame.ingest_calib_id(calib_id)
+        if len(_calib_id) == 1:
+            # There's only one calibration ID, so return it.  This works both
+            # for 'all' and for single-integer calibration groupings.
+            return _calib_id[0]
+
+        # Convert the IDs to integers and sort them
+        calibs = np.sort(np.array(_calib_id).astype(int))
+
+        # Find where the list is non-sequential
+        indx = np.diff(calibs) != 1
+        if not np.any(indx):
+            # The full list is sequential, so give the starting and ending points
+            return f'{calibs[0]}+{calibs[-1]}'
+
+        # Split the array into sequential subarrays (or single elements) and
+        # combine them into a single string
+        split_calibs = np.split(calibs, np.where(indx)[0]+1)
+        return '-'.join([f'{s[0]}+{s[-1]}' if len(s) > 1 else f'{s[0]}' for s in split_calibs])
+
+    @staticmethod
+    def parse_calib_id(calib_id_name):
+        """
+        Parse the calibration ID(s) from the unique string identifier used in
+        file naming.  I.e., this is the inverse of :func:`construct_calib_id`.
+
+        Args:
+            calib_id_name (:obj:`str`):
+                The string identifier used in file naming constructed from a
+                list of calibration IDs using :func:`construct_calib_id`.
+
+        Returns:
+            :obj:`list`: List of string representations of single calibration
+            group integer identifiers.
+        """
+        # Name is all, so we're done
+        if calib_id_name == 'all':
+            return ['all']
+        # Parse the name into slices and enumerate them
+        calib_id = []
+        for slc in calib_id_name.split('-'):
+            split_slc = slc.split('+')
+            calib_id += split_slc if len(split_slc) == 1 \
+                    else np.arange(int(split_slc[0]), int(split_slc[1])+1).astype(str).tolist()
+        return calib_id
+
     @staticmethod
     def construct_calib_key(setup, calib_id, detname):
         """
@@ -330,7 +398,7 @@ def construct_calib_key(setup, calib_id, detname):
         Returns:
             :obj:`str`: Calibration identifier.
         """
-        return f'{setup}_{"-".join(CalibFrame.ingest_calib_id(calib_id))}_{detname}'
+        return f'{setup}_{CalibFrame.construct_calib_id(calib_id)}_{detname}'
 
     @staticmethod
     def parse_calib_key(calib_key):
@@ -346,8 +414,8 @@ def parse_calib_key(calib_key):
         Returns:
             :obj:`tuple`: The three components of the calibration key.
         """
-        setup, calib_id, detname = calib_key.split('_')
-        return setup, ','.join(calib_id.split('-')), detname
+        setup, calib_id_name, detname = calib_key.split('_')
+        return setup, ','.join(CalibFrame.parse_calib_id(calib_id_name)), detname
 
     @classmethod
     def construct_file_name(cls, calib_key, calib_dir=None):

pypeit/tests/test_arcimage.py

@@ -57,4 +57,3 @@ def test_io():
     ofile.unlink()
 
 
-

pypeit/tests/test_calibframe.py

@@ -5,6 +5,8 @@
 
 from IPython import embed
 
+import numpy as np
+
 import pytest
 
 from pypeit.pypmsgs import PypeItError
@@ -56,7 +58,7 @@ def test_init():
 
     calib.set_paths(odir, 'A', ['1','2'], 'DET01')
     ofile = Path(calib.get_path()).name
-    assert ofile == 'Minimal_A_1-2_DET01.fits', 'Wrong file name'
+    assert ofile == 'Minimal_A_1+2_DET01.fits', 'Wrong file name'
 
 
 def test_io():
@@ -97,7 +99,7 @@ def test_construct_calib_key():
     key = CalibFrame.construct_calib_key('A', '1', 'DET01')
     assert key == 'A_1_DET01', 'Key changed'
     key = CalibFrame.construct_calib_key('A', ['1','2'], 'DET01')
-    assert key == 'A_1-2_DET01', 'Key changed'
+    assert key == 'A_1+2_DET01', 'Key changed'
     key = CalibFrame.construct_calib_key('A', 'all', 'DET01')
     assert key == 'A_all_DET01', 'Key changed'
 
@@ -115,6 +117,31 @@ def test_ingest_calib_id():
             'Bad ingest'
 
 
+def test_construct_calib_id():
+    assert CalibFrame.construct_calib_id(['all']) == 'all', 'Construction should simply return all'
+    assert CalibFrame.construct_calib_id(['1']) == '1', \
+            'Construction with one calib_id should just return it'
+    calib_id = np.arange(10).tolist()
+    assert CalibFrame.construct_calib_id(calib_id) == '0+9', 'Bad simple construction'
+    # rng = np.random.default_rng(99)
+    # calib_id = np.unique(rng.integers(20, size=15)).tolist()
+    calib_id = [3, 5, 6, 10, 11, 12, 15, 18, 19]
+    assert CalibFrame.construct_calib_id(calib_id) == '3-5+6-10+12-15-18+19', \
+            'Bad complex construction'
+
+
+def test_parse_calib_id():
+    assert CalibFrame.parse_calib_id('all') == ['all'], 'Parsing should simply return all'
+    assert CalibFrame.parse_calib_id('1') == ['1'], 'Parsing should simply return all'
+    assert np.array_equal(CalibFrame.parse_calib_id('0+9'), np.arange(10).astype(str).tolist()), \
+            'Bad simple construction'
+    # rng = np.random.default_rng(99)
+    # calib_id = np.unique(rng.integers(20, size=15)).tolist()
+    calib_id = np.sort(np.array([3, 5, 6, 10, 11, 12, 15, 18, 19]).astype(str))
+    assert np.array_equal(np.sort(CalibFrame.parse_calib_id('3-5+6-10+12-15-18+19')), calib_id), \
+            'Bad complex construction'
+
+
 def test_parse_key_dir():
     calib = MinimalCalibFrame()
     odir = Path(data_path('')).resolve()