Merge pull request #168 from desihub/ltdefault

Update default tiles file path in io.loadtiles()

.github/workflows/python-package.yml

@@ -45,8 +45,10 @@ jobs:
                 python -m pip install --no-deps --force-reinstall --ignore-installed 'fitsio${{ matrix.fitsio-version }}'
                 python -m pip install pyyaml requests scipy healpy matplotlib
                 svn export https://desi.lbl.gov/svn/code/desimodel/${DESIMODEL_DATA}/data
+                # ADM grab the surveyops directory.
+                wget -e robots=off -r -np -nH --cut-dirs 7 https://data.desi.lbl.gov/public/edr/survey/ops/surveyops/tags/0.1/ops/
             - name: Run the test
-              run: DESIMODEL=$(pwd) pytest
+              run: DESIMODEL=$(pwd) DESI_SURVEYOPS=$(pwd) pytest
 
     coverage:
         name: Test coverage
@@ -83,8 +85,10 @@ jobs:
                 python -m pip install --no-deps --force-reinstall --ignore-installed 'fitsio${{ matrix.fitsio-version }}'
                 python -m pip install pyyaml requests scipy healpy matplotlib
                 svn export https://desi.lbl.gov/svn/code/desimodel/${DESIMODEL_DATA}/data
+                # ADM grab the surveyops directory.
+                wget -e robots=off -r -np -nH --cut-dirs 7 https://data.desi.lbl.gov/public/edr/survey/ops/surveyops/tags/0.1/ops/
             - name: Run the test with coverage
-              run: DESIMODEL=$(pwd) pytest --cov
+              run: DESIMODEL=$(pwd) DESI_SURVEYOPS=$(pwd) pytest --cov
             - name: Coveralls
               env:
                 COVERALLS_REPO_TOKEN: ${{ secrets.COVERALLS_REPO_TOKEN }}

doc/changes.rst

@@ -5,7 +5,17 @@ desimodel Release Notes
 0.18.1 (unreleased)
 -------------------
 
-* No changes yet.
+* Change default file for :func:`~desimodel.io.loadtiles` (PR `#168`_):
+
+  * Now reads from :envvar:`DESI_SURVEYOPS` ``(/trunk/ops/tiles-main.ecsv)``
+  * Also adds option to limit tiles to specified ``PROGRAM`` names.
+  * Will now automatically load both `.fits` and `.ecsv` files.
+  * Maintains option to read from old :envvar:`DESIMODEL` location.
+  * Tests cover both :envvar:`DESI_SURVEYOPS` and :envvar:`DESIMODEL` cases.
+  * Addresses `issue #167`_.
+
+.. _`issue #167`: https://github.com/desihub/desimodel/issues/167
+.. _`#168`: https://github.com/desihub/desimodel/pull/168
 
 0.18.0 (2023-01-05)
 -------------------
@@ -249,11 +259,13 @@ Data changes to svn, no code changes:
 ------------------
 
 * Update data and associated code to reflect changes in DESI-347-v13 (PR `#89`_):
+
   * ``data/throughput/thru-[brz].fits``: new corrector coatings.
   * ``data/throughput/DESI-0347_blur.ecsv``: new achromatic blurs.
   * ``data/desi.yaml``: new read noise and dark currents.
   * ``data/focalplane/gfa.ecsv``: replace ``RADIUS_MM`` with ``S``.
   * ``data/throughput/DESI-0347_static_[123].fits``: replace random offset files (RMS=10.886um) with static offset files (RMS=8.0um).
+
 * Use a new svn branch test-0.9.6 for travis tests (was test-0.9.3).
 
 .. _`#89`: https://github.com/desihub/desimodel/pull/89

py/desimodel/footprint.py

@@ -22,18 +22,26 @@
 _pass2program = None
 
 
-def pass2program(tilepass):
+def pass2program(tilepass, surveyops=False):
     '''Converts integer tile pass number to string program name.
 
     Args:
         tilepass (int or int array): tiling pass number.
+    surveyops (bool): ``True`` to look for tiles in $DESI_SURVEYPOPS.
 
     Returns:
         Program name for each pass (str or list of str).
     '''
     global _pass2program
+    # ADM this function isn't useful if looking in the DESI_SURVEYOPS
+    # ADM directory as the real data is not ordered by pass.
+    if surveyops == True:
+        msg = "Function is not meaningful when looking in the DESI_SURVEYOPS "
+        msg += "directory as the real, Main Survey, data is not ordered by pass"
+        log.critical(msg)
+        raise ValueError(msg)
     if _pass2program is None:
-        tiles = load_tiles()
+        tiles = load_tiles(surveyops=False)
         _pass2program = dict(set(zip(tiles['PASS'], tiles['PROGRAM'])))
     if np.isscalar(tilepass):
         return _pass2program[tilepass]

py/desimodel/io.py

@@ -25,6 +25,11 @@
 _thru = dict()
 
 
+# ADM raise a custom exception when an environment variable is missing.
+class MissingEnvVar(Exception):
+    pass
+
+
 def load_throughput(channel):
     """Returns specter Throughput object for the given channel 'b', 'r', or 'z'.
 
@@ -214,8 +219,9 @@ def load_fiberpos():
 _tiles = dict()
 
 
-def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True):
-    """Return DESI tiles structure from ``$DESIMODEL/data/footprint/desi-tiles.fits``.
+def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True,
+               programs=None, surveyops=True):
+    """Return DESI tiles structure from ``$DESI_SURVEYOPS/trunk/ops/tiles-main.ecsv``.
 
     Parameters
     ----------
@@ -229,6 +235,12 @@ def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True):
     cache : :class:`bool`, optional
         If ``False``, force reload of data from tiles file, instead of
         using cached values.
+    programs : :class:`list` or `str`, optional
+        Pass a list of program names to restrict to only those programs,
+        e.g. ["DARK", "BACKUP"].
+    surveyops : :class:`bool`
+        If ``True`` then find the relevant path for the $DESI_SURVEYOPS
+        directory rather than the $DESIMODEL directory.
 
     Returns
     -------
@@ -252,29 +264,35 @@ def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True):
     If the parameter `tilesfile` is set, this function uses the following
     search method:
 
+    0. Paths corresponding to both $DESI_SURVEYOPS/trunk/ops and
+       $DESI_SURVEYOPS/ops are always both checked, to cover different
+       svn checkout approaches.
     1. If the value includes an explicit path, even ``./``, use that file.
-    2. If the value does *not* include an explicit path, *and* the file name
-       is identical to a file in ``$DESIMODEL/data/footprint/``, use the
-       file in ``$DESIMODEL/data/footprint/`` and issue a warning.
+    2. If the value does *not* include an explicit path, *and* the file
+       name is identical to a file in ``$DESI_SURVEYOPS/trunk/ops/``, use
+       the file in ``$DESI_SURVEYOPS/trunk/ops/`` and issue a warning.
     3. If no matching file can be found at all, raise an exception.
     """
     global _tiles
 
     if tilesfile is None:
         # Use the default
-        tilesfile = findfile("footprint/desi-tiles.fits")
+        if surveyops:
+            tilesfile = findfile("tiles-main.ecsv", surveyops=surveyops)
+        else:
+            tilesfile = findfile("footprint/desi-tiles.fits", surveyops=surveyops)
     else:
-        # If full path isn't included, check local vs $DESIMODEL/data/footprint
+        # If full path isn't included, check local vs $DESI_SURVEYOPS/ops
         tilepath, filename = os.path.split(tilesfile)
         if tilepath == "":
             have_local = os.path.isfile(tilesfile)
-            checkfile = findfile(os.path.join("footprint", tilesfile))
+            checkfile = findfile(tilesfile, surveyops=surveyops)
             have_dmdata = os.path.isfile(checkfile)
             if have_dmdata:
                 if have_local:
                     msg = (
-                        "$DESIMODEL/data/footprint/{0} is shadowed by a local"
-                        + " file. Choosing $DESIMODEL file."
+                        "$DESI_SURVEYOPS/(trunk)/ops/{0} is shadowed by a local"
+                        + " file. Choosing $DESI_SURVEYOPS file."
                         + ' Use tilesfile="./{0}" if you want the local copy'
                         + " instead."
                     ).format(tilesfile)
@@ -284,33 +302,39 @@ def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True):
             if not (have_local or have_dmdata):
                 msg = (
                     'File "{}" does not exist locally or in '
-                    + "$DESIMODEL/data/footprint/!"
+                    + "$DESI_SURVEYOPS/(trunk)/ops/!"
                 ).format(tilesfile)
                 raise FileNotFoundError(msg)
 
     # - standarize path location
     tilesfile = os.path.abspath(tilesfile.format(**os.environ))
     log.debug("Loading tiles from %s", tilesfile)
 
+    # ADM allow reading from either .fits or .ecsv files.
+    # ADM guard against the possibility that the file is zipped.
+    isfits = ".fits" in os.path.basename(tilesfile)
+
     if cache and tilesfile in _tiles:
         tiledata = _tiles[tilesfile]
     else:
-        with fits.open(tilesfile, memmap=False) as hdulist:
-            tiledata = hdulist[1].data
-        #
-        # Temporary workaround for problem identified in
-        # https://github.com/desihub/desimodel/issues/30
-        #
-        if any([c.bzero is not None for c in tiledata.columns]):
-            foo = [_tiles[k].dtype for k in tiledata.dtype.names]
-
-        # - Check for out-of-date tiles file
-        if np.issubdtype(tiledata["OBSCONDITIONS"].dtype, np.unsignedinteger):
-            warnings.warn(
-                "Old desi-tiles.fits with uint16 OBSCONDITIONS; please update your $DESIMODEL checkout.",
-                DeprecationWarning,
-            )
-
+        if isfits:
+            with fits.open(tilesfile, memmap=False) as hdulist:
+                tiledata = hdulist[1].data
+            #
+            # Temporary workaround for problem identified in
+            # https://github.com/desihub/desimodel/issues/30
+            #
+            if any([c.bzero is not None for c in tiledata.columns]):
+                foo = [_tiles[k].dtype for k in tiledata.dtype.names]
+
+            # - Check for out-of-date tiles file
+            if np.issubdtype(tiledata["OBSCONDITIONS"].dtype, np.unsignedinteger):
+                warnings.warn(
+                    "Old desi-tiles.fits with uint16 OBSCONDITIONS; please update your $DESIMODEL checkout.",
+                    DeprecationWarning,
+                )
+        else:
+            tiledata = Table.read(tilesfile)
         # - load cache for next time
         if cache:
             _tiles[tilesfile] = tiledata
@@ -324,6 +348,15 @@ def load_tiles(onlydesi=True, extra=False, tilesfile=None, cache=True):
     if not extra:
         subset &= ~np.char.startswith(tiledata["PROGRAM"], "EXTRA")
 
+    # ADM filter to program names if requested.
+    if programs is not None:
+        # ADM guard against a single string being passed.
+        programs = np.atleast_1d(programs)
+        isprog = np.zeros(len(tiledata), dtype=bool)
+        for program in programs:
+            isprog |= tiledata["PROGRAM"] == program
+        subset &= isprog
+
     if np.all(subset):
         return tiledata
     else:
@@ -690,14 +723,18 @@ def load_pixweight(nside, pixmap=None):
     return hp.pixelfunc.ud_grade(pixmap, nside, order_in="NESTED", order_out="NESTED")
 
 
-def findfile(filename):
+def findfile(filename, surveyops=False):
     """Return full path to data file ``$DESIMODEL/data/filename``.
 
     Parameters
     ----------
     filename : :class:`str`
         Name of the file, relative to the desimodel data directory.
 
+    surveyops : :class:`bool`
+        If ``True`` then find the relevant path for the $DESI_SURVEYOPS
+        directory rather than the $DESIMODEL directory.
+
     Returns
     -------
     :class:`str`
@@ -709,17 +746,37 @@ def findfile(filename):
     desimodel data would be installed with the package and :envvar:`DESIMODEL`
     would become an optional override.
     """
-    return os.path.join(datadir(), filename)
+    return os.path.join(datadir(surveyops), filename)
 
 
-def datadir():
+def datadir(surveyops=False):
     """Returns location to desimodel data.
 
-    If set, :envvar:`DESIMODEL` overrides data installed with the package.
+    Parameters
+    ----------
+    surveyops : :class:`bool`
+        If ``True`` then find the relevant path for the $DESI_SURVEYOPS
+        directory rather than the $DESIMODEL directory.
+
+    Notes
+    -----
+    If `surveyops`==``False`` and :envvar:`DESIMODEL` is set, then
+    $DESIMODEL overrides data installed with the package.
     """
-    if "DESIMODEL" in os.environ:
-        return os.path.abspath(os.path.join(os.environ["DESIMODEL"], "data"))
+    if surveyops:
+        if "DESI_SURVEYOPS" in os.environ:
+            surveyops = os.environ["DESI_SURVEYOPS"]
+            # ADM test whether surveyops directory was checked out to trunk.
+            if os.path.isdir(os.path.join(surveyops, "trunk", "ops")):
+                surveyops = os.path.join(surveyops, "trunk")
+            return os.path.abspath(os.path.join(surveyops, "ops"))
+        # ADM raise a custom exception if $DESI_SURVEYOPS is not set.
+        else:
+            raise MissingEnvVar(f"$DESI_SURVEYOPS is not set")
     else:
-        import pkg_resources
+        if "DESIMODEL" in os.environ:
+            return os.path.abspath(os.path.join(os.environ["DESIMODEL"], "data"))
+        else:
+            import pkg_resources
 
-        return pkg_resources.resource_filename("desimodel", "data")
+            return pkg_resources.resource_filename("desimodel", "data")

py/desimodel/test/test_footprint.py

@@ -51,18 +51,22 @@ def test_pass2program(self):
         with self.assertRaises(KeyError):
             footprint.pass2program(999)
 
+
     def test_program2pass(self):
         '''Test footprint.program2pass().
         '''
-        self.assertEqual(len(footprint.program2pass('DARK')), 4)
-        self.assertEqual(len(footprint.program2pass('GRAY')), 1)
-        self.assertEqual(len(footprint.program2pass('BRIGHT')), 3)
-
-        passes = footprint.program2pass(['DARK', 'GRAY', 'BRIGHT'])
+        self.assertEqual(len(footprint.program2pass('DARK')), 7)
+        # ADM in the real survey data there is no GRAY program...
+#        self.assertEqual(len(footprint.program2pass('GRAY')), 1)
+        # ADM ...but there is a BACKUP program.
+        self.assertEqual(len(footprint.program2pass('BACKUP')), 1)
+        self.assertEqual(len(footprint.program2pass('BRIGHT')), 4)
+
+        passes = footprint.program2pass(['DARK', 'BACKUP', 'BRIGHT'])
         self.assertEqual(len(passes), 3)
-        self.assertEqual(len(passes[0]), 4)
+        self.assertEqual(len(passes[0]), 7)
         self.assertEqual(len(passes[1]), 1)
-        self.assertEqual(len(passes[2]), 3)
+        self.assertEqual(len(passes[2]), 4)
 
         with self.assertRaises(ValueError):
             footprint.program2pass('BLAT')