Merge pull request #265 from sot/ruff-pyright

Refresh regression testing and fix ruff / improve variable naming

Author: taldcroft

.pre-commit-config.yaml

@@ -1,7 +1,7 @@
 repos:
 - repo: https://github.com/astral-sh/ruff-pre-commit
   # Ruff version.
-  rev: v0.3.3
+  rev: v0.8.0
   hooks:
     # Run the linter.
     - id: ruff

NOTES/NOTES.test

@@ -1,24 +1,25 @@
-##########################################################################
-# For py.test unit tests and general development
-##########################################################################
-skadev
-export KADI=/proj/sot/ska/data/kadi
-export ENG_ARCHIVE=/proj/sot/ska/data/eng_archive
-
-python setup.py install
-cd doc  # make sure it's picking up the installed version
-ipython
->>> import Ska.engarchive
->>> Ska.engarchive.test(args='-k -test_fetch_regr')  # normally skip this
-
-##########################################################################
-# Build and install docs
-##########################################################################
-cd doc
-make html
-cd ..
-su -l aca
-make install_doc
+#################################################################################
+# Regression test for primary (HEAD) update_archive.py and fetch.py
+#################################################################################
+
+# This script generates a fresh cheta data archive using the current code in the
+# repository. It then compares the outputs for a handful of MSIDs to flight archive.
+# It should take a few minutes to run.
+
+./regression_test_eng_archive.sh
+
+# Basic sanity test with some plots (probably not useful)
+
+$ env ENG_ARCHIVE=$PWD/test/eng_archive ipython --matplotlib
+>>> run test/make_plots.py
+
+#############################################################################
+#############################################################################
+# Everything below here is likely out of date and/or not useful.            #
+# It is included strictly for reference just in case.                       #
+#############################################################################
+#############################################################################
+
 
 #############################################################################
 # Make baseline (seed) test data using flight Ska.
@@ -44,82 +45,11 @@ tar zcf eng_archive.tar.gz eng_archive
 cp eng_archive.tar.gz /proj/sot/ska/data/eng_archive/regr/flight_eng_archive.tar.gz
 popd
 
-
-#################################################################################
-######## Regression test for primary (HEAD) update_archive.py and fetch.py
-#################################################################################
-
-# First make test data.  This sets ENG_ARCHIVE in that window.
-
-[[ source make_test_eng_archive.csh ]]
-. make_test_eng_archive.sh   # THIS actually works now
-
-# Basic sanity test with some plots
-
-[[ setenv ENG_ARCHIVE $PWD/test/eng_archive ]]
-export ENG_ARCHIVE=$PWD/test/eng_archive
-
-test/make_plots.py
-
-# Regression test.  Get_regr_vals.py uses the local ../Ska/engarchive/fetch.py
-# if --test is given, otherwise uses the installed Ska version.  It sets
-# ENG_ARCHIVE internally prior to importing fetch.
-
-[[ unsetenv ENG_ARCHIVE ]]
-unset ENG_ARCHIVE
-
-cd test
-  ./get_regr_vals.py --start 2012:302 --stop 2012:310
-  ./get_regr_vals.py --start 2012:302 --stop 2012:310 --test
-  ./compare_regr_vals.py
-cd ..
-[[ setenv ENG_ARCHIVE $PWD/test/eng_archive ]]
-export ENG_ARCHIVE=$PWD/test/eng_archive
-
-## 2011-Feb-18: All tests pass for rev 56:5b12e819e1df
-## 2012-Jan-21: All tests pass for 0.16dev e359acd5 !!
-## 2012-Apr-29: All tests pass for 0.18dev efddf4d
-## 2012-Nov-27: All tests pass for 0.21
-
-############################################################################################
-######## Regression test for new skare (updated Python, NumPy, tables, etc) in $ska/test/
-############################################################################################
-
-skatest
-
-. make_test_eng_archive.sh   # THIS actually works now
-
-# Basic sanity test with some plots
-test/make_plots.py
-
-pushd test
-  rm regr_vals.test
-  ./get_regr_vals.py --start 2012:302 --stop 2012:310 --test
-popd
-
-
-############
-# Regression test.  Get_regr_vals.py uses the local ../Ska/engarchive/fetch.py
-# if --test is given, otherwise uses the installed Ska version.  It sets
-# ENG_ARCHIVE internally prior to importing fetch.
-ska
-unset ENG_ARCHIVE
-pushd test
-  rm regr_vals.flight
-  ./get_regr_vals.py --start 2012:302 --stop 2012:310
-  ./compare_regr_vals.py
-popd
-
-## 2011-Jun-20 All tests pass for devska (skare 0.10)
-## 2012-Jun-12 All tests pass in skatest (skare 0.13) + new eng archive ddc63d2
-## 2012-Jun-20 All tests pass in skatest skare 0.13-r241-427bb9c + eng_arch ddc63d2
-## 2013-Apr-16 All tests pass in skatest skare 0.15-r293--e754375 + eng_arch f5db57c
-
 ########################################################################################################
 ######## Regression test for new skare in /proj/sot/ska
 ########################################################################################################
 
-# Make baseline (seed) test data using flight Ska. 
+# Make baseline (seed) test data using flight Ska.
 
 ska
 
@@ -178,7 +108,7 @@ test/make_plots.py
 
 unsetenv ENG_ARCHIVE
 pushd test
-./get_regr_vals.py --start 2010:265 --stop 2010:285 --test 
+./get_regr_vals.py --start 2010:265 --stop 2010:285 --test
 ./get_regr_vals.py --start 2010:265 --stop 2010:285
 ./compare_regr_vals.py
 popd

cheta/derived/comps.py

@@ -435,8 +435,8 @@ def get_msid_attrs(self, tstart: float, tstop: float, msid: str, msid_args: tupl
             )
             quat = Quat(q=normalize(q))
         bads = np.zeros_like(q1, dtype=bool)
-        for msid in msids:
-            bads |= dat[msid].bads
+        for msid_ in msids:
+            bads |= dat[msid_].bads
 
         out = {"vals": quat, "bads": bads, "times": dat.times, "unit": None}
         return out

cheta/tests/test_orbit.py

@@ -32,5 +32,5 @@ def test_orbital_elements():
         "mean_anomaly": 354.971325,
     }  # 354.9724 in reference (roundoff in example)
 
-    for key in expected:
+    for key in expected:  # noqa: PLC0206
         assert np.allclose(out[key], expected[key], atol=0.0, rtol=1e-6)

cheta/update_archive.py

@@ -920,21 +920,43 @@ def truncate_archive(filetype, date):
     logger.verbose(cmd)
 
 
-def read_archfile(i, f, filetype, row, colnames, archfiles, db):
-    """Read filename ``f`` with index ``i`` (position within list of filenames).  The
-    file has type ``filetype`` and will be added to MSID file at row index ``row``.
-    ``colnames`` is the list of column names for the content type (not used here).
+def read_archfile(idx_archfile, archfile, filetype, row, colnames, archfiles, db):
+    """Read a FITS file which has been retrieved from the CXCDS archive.
+
+    Parameters
+    ----------
+    idx_archfile : int
+        Index of the archfile in the list of archfiles
+    archfile : str
+        Full path of FITS file to read
+    filetype : dict
+        Filetype dictionary with keys, level, instrum, content, arc5gl_query, fileglob.
+    row : int
+        Row number in the MSID file to start writing data
+    colnames : list of str
+        List of column names for the content type
+    archfiles : numpy structured array
+        Array of archfiles
+
+    Returns
+    -------
+    dat : numpy structured array
+        Data read from the FITS file
+    archfiles_row : dict
+        Row of info about this FILE to insert into archfiles table for content type
     """
     # Check if filename is already in archfiles.  If so then abort further processing.
-    filename = os.path.basename(f)
+    filename = os.path.basename(archfile)
     if db.fetchall("SELECT filename FROM archfiles WHERE filename=?", (filename,)):
-        logger.verbose("File %s already in archfiles - unlinking and skipping" % f)
-        os.unlink(f)
+        logger.verbose(
+            "File %s already in archfiles - unlinking and skipping" % archfile
+        )
+        os.unlink(archfile)
         return None, None
 
     # Read FITS archive file and accumulate data into dats list and header into headers dict
-    logger.info("Reading (%d / %d) %s" % (i, len(archfiles), filename))
-    hdus = pyfits.open(f, character_as_bytes=True)
+    logger.info("Reading (%d / %d) %s" % (idx_archfile, len(archfiles), filename))
+    hdus = pyfits.open(archfile, character_as_bytes=True)
     hdu = hdus[1]
 
     try:
@@ -975,14 +997,33 @@ def read_archfile(i, f, filetype, row, colnames, archfiles, db):
     return dat, archfiles_row
 
 
-def read_derived(i, filename, filetype, row, colnames, archfiles, db):
+def read_derived(idx_archfile, filename, filetype, row, colnames, archfiles, db):
     """Read derived data using eng_archive and derived computation classes.
-    ``filename`` has format <content>_<index0>_<index1> where <content>
-    is the content type (e.g. "dp_thermal128"), <index0> is the start index for
-    the new data and index1 is the end index (using Python slicing convention
-    index0:index1).  Args ``i``, ``filetype``, and ``row`` are as in
-    read_archive().  ``row`` must equal <index0>.  ``colnames`` is the list of
-    column names for the content type.
+
+    Parameters
+    ----------
+    idx_archfile : int
+        Index of the archfile in the list of archfiles
+    filename : str
+        File to read with format <content>_<index0>_<index1> where <content> is the
+        content type (e.g. "dp_thermal128"), <index0> is the start index for the new
+        data and <index1> is the end index (using Python slicing convention
+        index0:index1).
+    filetype : dict
+        Filetype dictionary with keys, level, instrum, content, arc5gl_query, fileglob.
+    row : int
+        Row number in the MSID file to start writing data (must equal <index0>)
+    colnames : list of str
+        List of column names for the content type
+    archfiles : numpy structured array
+        Array of archfiles
+
+    Returns
+    -------
+    dat : numpy structured array
+        Data read from the FITS file
+    archfiles_row : dict
+        Row of info about this FILE to insert into archfiles table for content type
     """
     # Check if filename is already in archfiles.  If so then abort further processing.
 
@@ -999,20 +1040,20 @@ def read_derived(i, filename, filetype, row, colnames, archfiles, db):
     time_step = mnf_step * cheta.derived.MNF_TIME
     times = time_step * np.arange(index0, index1)
 
-    logger.info("Reading (%d / %d) %s" % (i, len(archfiles), filename))
+    logger.info("Reading (%d / %d) %s" % (idx_archfile, len(archfiles), filename))
     vals = {}
     bads = np.zeros((len(times), len(colnames)), dtype=bool)
-    for i, colname in enumerate(colnames):
+    for ii, colname in enumerate(colnames):
         if colname == "TIME":
             vals[colname] = times
-            bads[:, i] = False
+            bads[:, ii] = False
         else:
             dp_class = getattr(cheta.derived, colname.upper())
             dp = dp_class()
             dataset = dp.fetch(times[0] - 1000, times[-1] + 1000)
             ok = (index0 <= dataset.indexes) & (dataset.indexes < index1)
             vals[colname] = dp.calc(dataset)[ok]
-            bads[:, i] = dataset.bads[ok]
+            bads[:, ii] = dataset.bads[ok]
 
     vals["QUALITY"] = bads
     dat = Ska.Numpy.structured_array(vals, list(colnames) + ["QUALITY"])
@@ -1059,9 +1100,11 @@ def update_msid_files(filetype, archfiles):
 
     content_is_derived = filetype["instrum"] == "DERIVED"
 
-    for i, f in enumerate(archfiles):
+    for idx_archfile, archfile in enumerate(archfiles):
         get_data = read_derived if content_is_derived else read_archfile
-        dat, archfiles_row = get_data(i, f, filetype, row, colnames, archfiles, db)
+        dat, archfiles_row = get_data(
+            idx_archfile, archfile, filetype, row, colnames, archfiles, db
+        )
         if dat is None:
             continue
 
@@ -1154,7 +1197,7 @@ def update_msid_files(filetype, archfiles):
         # subsequent relocation into arch_files archive.  In the case of a gap
         # where ingest is stopped before all archfiles are processed, this will
         # leave files in a tmp dir.
-        archfiles_processed.append(f)
+        archfiles_processed.append(archfile)
         if not opt.dry_run:
             db.insert(archfiles_row, "archfiles")
 

cheta/update_server_sync.py

@@ -446,8 +446,7 @@ def _get_stat_data_from_archive(filename, stat, tstart, tstop, last_row1, logger
 
             # For rarely sampled data like CPE1ENG, delta_rows can end up being larger than the
             # table due to the gaps.  Therefore clip to the length of the table.
-            if delta_rows > len(table):
-                delta_rows = len(table)
+            delta_rows = min(delta_rows, len(table))
 
             times = (table[-delta_rows:]["index"] + 0.5) * dt