Merge pull request #17 from lsst/tickets/DM-36472

DM-36472: Expose validateGalsimInterpolant and fix a bug in it

.github/workflows/lint.yml

@@ -1,22 +1,11 @@
 name: lint
 
 on:
-  - push
-  - pull_request
+  push:
+    branches:
+      - main
+  pull_request:
 
 jobs:
-  lint:
-    runs-on: ubuntu-latest
-    steps:
-      - uses: actions/checkout@v2
-
-      - name: Set up Python
-        uses: actions/setup-python@v2
-        with:
-          python-version: 3.7
-
-      - name: Install
-        run: pip install -r <(curl https://raw.githubusercontent.com/lsst/linting/main/requirements.txt)
-
-      - name: Run linter
-        run: flake8
+  call-workflow:
+    uses: lsst/rubin_workflows/.github/workflows/lint.yaml@main

python/lsst/meas/extensions/piff/piffPsfDeterminer.py

@@ -32,40 +32,41 @@
 from .piffPsf import PiffPsf
 
 
-class PiffPsfDeterminerConfig(BasePsfDeterminerTask.ConfigClass):
-    def _validateGalsimInterpolant(name: str) -> bool:  # noqa: N805
-        """A helper function to validate the GalSim interpolant at config time.
+def _validateGalsimInterpolant(name: str) -> bool:
+    """A helper function to validate the GalSim interpolant at config time.
 
-        Parameters
-        ----------
-        name : str
-            The name of the interpolant to use from GalSim.  Valid options are:
-                Lancsos(N) where n is a positive integer
-                Linear
-                Cubic
-                Quintic
-                Delta
-                Nearest
-                SincInterpolant
+    Parameters
+    ----------
+    name : str
+        The name of the interpolant to use from GalSim.  Valid options are:
+            galsim.Lanczos(N) or Lancsos(N), where N is a positive integer
+            galsim.Linear
+            galsim.Cubic
+            galsim.Quintic
+            galsim.Delta
+            galsim.Nearest
+            galsim.SincInterpolant
 
-        Returns
-        -------
-        is_valid : bool
-            Whether the provided interpolant name is valid.
-        """
-        # First, check if ``name`` is a valid Lanczos interpolant.
-        for pattern in (re.compile(r"Lanczos\(\d+\)"), re.compile(r"galsim.Lanczos\(\d+\)"),):
-            match = re.match(pattern, name)  # Search from the start of the string.
-            if match is not None:
-                # Check that the pattern is also the end of the string.
-                return match.end() == len(name)
-
-        # If not, check if ``name`` is any other valid GalSim interpolant.
-        names = {"galsim.{interp}" for interp in
-                 ("Cubic", "Delta", "Linear", "Nearest", "Quintic", "SincInterpolant")
-                 }
-        return name in names
+    Returns
+    -------
+    is_valid : bool
+        Whether the provided interpolant name is valid.
+    """
+    # First, check if ``name`` is a valid Lanczos interpolant.
+    for pattern in (re.compile(r"Lanczos\(\d+\)"), re.compile(r"galsim.Lanczos\(\d+\)"),):
+        match = re.match(pattern, name)  # Search from the start of the string.
+        if match is not None:
+            # Check that the pattern is also the end of the string.
+            return match.end() == len(name)
 
+    # If not, check if ``name`` is any other valid GalSim interpolant.
+    names = {f"galsim.{interp}" for interp in
+             ("Cubic", "Delta", "Linear", "Nearest", "Quintic", "SincInterpolant")
+             }
+    return name in names
+
+
+class PiffPsfDeterminerConfig(BasePsfDeterminerTask.ConfigClass):
     spatialOrder = pexConfig.Field(
         doc="specify spatial order for PSF kernel creation",
         dtype=int,

tests/test_psf.py

@@ -1,6 +1,8 @@
+import galsim  # noqa: F401
 import unittest
 import numpy as np
 import copy
+from galsim import Lanczos  # noqa: F401
 
 import lsst.utils.tests
 import lsst.afw.detection as afwDetection
@@ -13,6 +15,7 @@
 import lsst.meas.algorithms as measAlg
 from lsst.meas.base import SingleFrameMeasurementTask
 from lsst.meas.extensions.piff.piffPsfDeterminer import PiffPsfDeterminerConfig, PiffPsfDeterminerTask
+from lsst.meas.extensions.piff.piffPsfDeterminer import _validateGalsimInterpolant
 
 
 def psfVal(ix, iy, x, y, sigma1, sigma2, b):
@@ -335,6 +338,29 @@ def test_validatePsfCandidates(self, samplingSize):
         self.psfDeterminer._validatePsfCandidates(psfCandidateList, 21, samplingSize)
 
 
+class PiffConfigTestCase(lsst.utils.tests.TestCase):
+    """A test case to check for valid Piff config"""
+    def testValidateGalsimInterpolant(self):
+        # Check that random strings are not valid interpolants.
+        self.assertFalse(_validateGalsimInterpolant("foo"))
+        # Check that the Lanczos order is an integer
+        self.assertFalse(_validateGalsimInterpolant("Lanczos(3.0"))
+        self.assertFalse(_validateGalsimInterpolant("Lanczos(-5.0"))
+        self.assertFalse(_validateGalsimInterpolant("Lanczos(N)"))
+        # Check for various valid Lanczos interpolants
+        for interp in ("Lanczos(4)", "galsim.Lanczos(7)"):
+            self.assertTrue(_validateGalsimInterpolant(interp))
+            self.assertFalse(_validateGalsimInterpolant(interp.lower()))
+            # Evaluating the string should succeed. This is how Piff does it.
+            self.assertTrue(eval(interp))
+        # Check that interpolation methods are case sensitive.
+        for interp in ("Linear", "Cubic", "Quintic", "Delta", "Nearest", "SincInterpolant"):
+            self.assertFalse(_validateGalsimInterpolant(f"galsim.{interp.lower()}"))
+            self.assertFalse(_validateGalsimInterpolant(interp))
+            self.assertTrue(_validateGalsimInterpolant(f"galsim.{interp}"))
+            self.assertTrue(eval(f"galsim.{interp}"))
+
+
 class TestMemory(lsst.utils.tests.MemoryTestCase):
     pass