Merge pull request #193 from iraf-community/issue192

Fix the use of Pset arguments when calling a task in PyRAF CL

Author: olebole

.github/workflows/citest.yml

@@ -18,6 +18,7 @@ jobs:
           - name: Ubuntu 24.04 (Python-3.12.3), native
             os: ubuntu-24.04
             method: native
+            iraf: /usr/lib/iraf/
             upload_coverage: yes
 
           - name: Ubuntu 24.04 (Python-3.12.3), native, without IRAF
@@ -26,6 +27,7 @@ jobs:
 
           - name: Ubuntu 22.04 (Python-3.10), pip
             os: ubuntu-22.04
+            iraf: /usr/lib/iraf/
             method: pip
 
           - name: macOS 26, pip
@@ -37,11 +39,11 @@ jobs:
         if: matrix.method == 'native'
         uses: actions/checkout@v6
 
-      - name: Setup dependencies
+      - name: Setup dependencies, Ubuntu/native
         if: startsWith(matrix.os, 'ubuntu') && matrix.method == 'native'
         run: |
           sudo apt-get update
-          if [ "$iraf" ]; then
+          if [ "${{ matrix.iraf }}" ]; then
               sudo apt-get install --no-install-recommends iraf iraf-noao iraf-dev
           else
               echo "PYRAF_NO_IRAF=yes" >> $GITHUB_ENV
@@ -50,7 +52,7 @@ jobs:
           sudo apt-get install --no-install-recommends python3-dev python3-pip python3-astropy python3-numpy-dev python3-setuptools python3-setuptools-scm python3-tk python3-pytest python3-pytest-cov ipython3
           pip3 install "stsci.tools>=4.0.1" coveragepy
 
-      - name: Setup dependencies
+      - name: Setup dependencies, Ubuntu/pip
         if: startsWith(matrix.os, 'ubuntu') && matrix.method == 'pip'
         run: |
           sudo apt-get update

pyraf/irafpar.py

@@ -1038,6 +1038,18 @@ def setParList(self, *args, **kw):
                 p = p.get().getParObject(tail)
                 # what if *this* p is a IrafParPset ? skip for now,
                 # since we think no one is doubly nesting PSETs
+
+                # The original parameter in the pardict is not linked
+                # to the child Pset, so we need to retrieve the
+                # pardict one.
+                p.set(value)
+                p.setFlags(_cmdlineFlag)
+                try:
+                    p = self.getParObject(p.name)
+                except KeyError:
+                    p = copy.deepcopy(p)
+                    self.addParam(p)
+
             p.set(value)
             p.setFlags(_cmdlineFlag)
             if p.mode != "h":

pyraf/tests/test_cl_param.py

@@ -0,0 +1,139 @@
+import os
+from pathlib import Path
+from io import StringIO
+import shutil
+import pytest
+
+from .utils import HAS_IRAF
+
+from pyraf import iraf
+
+try:
+    from iraf import softools
+except ImportError:
+    pytest.skip("IRAF softools not available", allow_module_level=True)
+
+
+@pytest.fixture(scope="session", autouse=True)
+def setup_testpkg(tmp_path_factory):
+    """Copy the test package to a temporary dir, compile the SPP tasks and
+    load the package
+
+    """
+    base = tmp_path_factory.mktemp("iraf")
+    for fname in (Path(__file__).parent / "testpkg").iterdir():
+        shutil.copy(fname, base)
+    iraf.set(testpkg=str(base) + "/")
+    cwd = Path.cwd()
+    os.chdir(base)
+    try:
+        iraf.xc("spppars.x")
+    finally:
+        os.chdir(cwd)
+    iraf.task(testpkgDOTpkg="testpkg$testpkg.cl")
+    from iraf import testpkg
+
+    yield
+
+
+def showpars(taskname, **kwargs):
+    """Run the task with given parameters
+
+    Return the output as a dictionary of key-value pairs with proper
+    type for the value
+
+    """
+    task = iraf.module.getTask(taskname)
+    params = task.getParDict()
+    stdout = StringIO()
+    task(StdoutAppend=stdout, **kwargs)
+    stdout.seek(0)
+    res = {}
+    for l in stdout.read().splitlines():
+        key, val = l.split("=", 1)
+        res[key] = params[key].checkValue(val)
+    return res
+
+
+@pytest.mark.parametrize(
+    "name,value",
+    [
+        ("strpar", "foo.fits"),
+        ("intpar", 2),
+        ("intpar", -2),
+        ("fltpar", 2.0),
+        ("fltpar", 2.0e1),
+        ("fltpar", 2),
+        ("boolpar", True),
+        ("boolpar", 1),
+    ],
+)
+@pytest.mark.parametrize("task", ["clpars", "simplepars"])
+def test_simplepars(task, name, value):
+    """Check that simple parameters work for CL and SPP tasks"""
+    ref = showpars(task)
+    shortname = name.split(".", 1)[-1]
+    ref[shortname] = value
+    assert ref == showpars(task, **{name: value})
+
+
+@pytest.mark.parametrize(
+    "name,value",
+    [
+        ("psetpar.psintpar", 3),
+        ("psintpar", 3),
+        ("psetpar.psstrpar", "bar"),
+        ("psstrpar", "bar"),
+    ],
+)
+@pytest.mark.parametrize("task", ["clpars", "psetpars0", "psetpars1"])
+def test_pset(task, name, value):
+    """Check that parameter sets for for CL and SPP tasks"""
+    ref = showpars(task)
+    shortname = name.split(".", 1)[-1]
+    ref[shortname] = value
+    assert ref == showpars(task, **{name: value})
+
+
+@pytest.mark.xfail(reason="Not implemented in PyRAF")
+@pytest.mark.parametrize("task", ["clpars", "psetpars0", "psetpars1"])
+def test_pars_other_pset(task):
+    """Check that setting another parameter set works in CL and SPP tasks"""
+    ref = showpars(task)
+    ref["psstrpar"] = "foobar"
+    ref["psintpar"] = -1
+    assert ref == showpars(task, psetpar="psetpar1")
+
+
+@pytest.mark.parametrize(
+    "name,value",
+    [
+        ("intpar", "-20"),
+        ("intpar", "20"),
+        ("intpar", "foo"),
+        ("fltpar", "-100.5"),
+        ("fltpar", "1e3"),
+        ("fltpar", "foo"),
+        ("boolpar", "foo"),
+        ("boolpar", -99),
+    ],
+)
+@pytest.mark.parametrize("task", ["clpars", "simplepars"])
+def test_pars_invalid_value(task, name, value):
+    """Check that invalid values raise a ValueError"""
+    with pytest.raises(ValueError):
+        showpars(task, **{name: value})
+
+
+@pytest.mark.parametrize(
+    "name,value",
+    [
+        ("invalidpar", "foo"),
+        ("psetpar", "foo"),
+    ],
+)
+@pytest.mark.parametrize("task", ["clpars", "simplepars"])
+def test_pars_invalid_name(task, name, value):
+    """Check that unknown parameters raise a KeyError"""
+    with pytest.raises(KeyError):
+        showpars(task, **{name: value})

pyraf/tests/testpkg/clpars.cl

@@ -0,0 +1,10 @@
+# Procedure clpars: CL script to just print the parameters
+procedure clpars(strpar, intpar, fltpar, boolpar, psetpar)
+begin
+    print ("strpar=", strpar)
+    print ("intpar=", intpar)
+    print ("fltpar=", fltpar)
+    print ("boolpar=", boolpar)
+    print ("psstrpar=", psetpar.psstrpar)
+    print ("psintpar=", psetpar.psintpar)
+end

pyraf/tests/testpkg/clpars.par

@@ -0,0 +1,5 @@
+strpar,s,h,"unset",,,"String parameter"
+intpar,i,h,1,-10,10,"Integer parameter"
+fltpar,r,h,1.0,-100,100,"Floating point parameter"
+boolpar,b,h,no,,,"Boolean parameter"
+psetpar,pset,h,,,,"Parameter set (pset)"

pyraf/tests/testpkg/psetpar.par

@@ -0,0 +1,2 @@
+psstrpar,s,h,"",,,"String par in pset"
+psintpar,i,h,0,,,"Integer par in pset"

pyraf/tests/testpkg/psetpar1.par

@@ -0,0 +1,2 @@
+psstrpar,s,h,"foobar",,,"String par in pset"
+psintpar,i,h,-1,,,"Integer par in pset"

pyraf/tests/testpkg/psetpars0.par

@@ -0,0 +1,2 @@
+simple,b,h,no,,,"Use simplified parameter set access"
+psetpar,pset,h,,,,"Parameter set (pset)"

pyraf/tests/testpkg/psetpars1.par

@@ -0,0 +1 @@
+psetpars0.par

pyraf/tests/testpkg/simplepars.par

@@ -0,0 +1,5 @@
+strpar,s,h,"foo",,,"String parameter"
+intpar,i,h,1,-10,10,"Integer parameter"
+fltpar,r,h,1.0,-100,100,"Floating point parameter"
+boolpar,b,h,no,,,"Boolean parameter"
+