Merge branch 'rf/fslsub' into 'master'

Add fsl.wrappers.fsl_sub, deprecate fsl.utils.fslsub.submit

Closes #376 and #373

See merge request fsl/fslpy!309

CHANGELOG.rst

@@ -2,6 +2,47 @@ This document contains the ``fslpy`` release history in reverse chronological
 order.
 
 
+3.7.0 (Under development)
+-------------------------
+
+
+Added
+^^^^^
+
+
+* New :mod:`fsl.wrappers.fsl_sub` wrapper function for the ``fsl_sub``
+  command.
+
+
+Changed
+^^^^^^^
+
+* The default behaviour of the :func:`fsl.utils.run.run` function (and hence
+  that of all :mod:`fsl.wrappers` functions) has been changed so that the
+  standard output and error of the called command is now forwarded to the
+  calling Python process, in addition to being returned from ``run`` as
+  strings. In other words, the default behaviour of ``run('cmd')``, is now
+  equivalent to ``run('cmd', log={"tee":True})``. The previous default
+  behaviour can be achieved with ``run('cmd', log={"tee":False})``.
+* The :func:`fsl.utils.run.run` and :func:`fsl.utils.run.runfsl` functions
+  (and hence all :mod:`fsl.wrappers` functions) have been modified to use
+  ``fsl.wrappers.fsl_sub`` instead of ``fsl.utils.fslsub.submit``. This is an
+  internal change which should not affect the usage of the ``run``, ``runfsl``
+  or wrapper functions.
+
+
+Deprecated
+^^^^^^^^^^
+
+
+* :class:`fsl.utils.fslsub.SubmitParams` and :func:`fsl.utils.fslsub.submit`
+  have been deprecated in favour of using the ``fsl.wrappers.fsl_sub`` wrapper
+  function.
+* The :func:`fsl.utils.fslsub.info` function has been deprecated in favour of
+  using the ``fsl_sub.report`` function, from the separate `fsl_sub
+  <https://git.fmrib.ox.ac.uk/fsl/fsl_sub>`_ Python library.
+
+
 3.6.4 (Tuesday 3rd August 2021)
 -------------------------------
 

doc/fsl.wrappers.epi_reg.rst

+``fsl.wrappers.epi_reg``
+========================
+
+.. automodule:: fsl.wrappers.epi_reg
+    :members:
+    :undoc-members:
+    :show-inheritance:

doc/fsl.wrappers.fsl_sub.rst

+``fsl.wrappers.fsl_sub``
+========================
+
+.. automodule:: fsl.wrappers.fsl_sub
+    :members:
+    :undoc-members:
+    :show-inheritance:

doc/fsl.wrappers.rst

@@ -6,12 +6,14 @@
 
    fsl.wrappers.bet
    fsl.wrappers.eddy
+   fsl.wrappers.epi_reg
    fsl.wrappers.fast
    fsl.wrappers.flirt
    fsl.wrappers.fnirt
+   fsl.wrappers.fsl_anat
+   fsl.wrappers.fsl_sub
    fsl.wrappers.fslmaths
    fsl.wrappers.fslstats
-   fsl.wrappers.fsl_anat
    fsl.wrappers.fugue
    fsl.wrappers.melodic
    fsl.wrappers.misc

fsl/utils/fslsub.py

@@ -53,13 +53,19 @@ import warnings
 import os
 
 
+import fsl.utils.deprecated as deprecated
+
+
 log = logging.getLogger(__name__)
 
 
 @dataclass
 class SubmitParams(object):
-    """
-    Represents the fsl_sub parameters
+    """Represents the fsl_sub parameters
+
+    The ``SubmitParams`` class is deprecated - you should use
+    :mod:`fsl.wrappers.fsl_sub` instead, or use the ``fsl_sub`` Python
+    library, which is installed as part of FSL.
 
     Any command line script can be submitted by the parameters by calling the `SubmitParams` object:
 
@@ -152,6 +158,8 @@ class SubmitParams(object):
     def __str__(self):
         return 'SubmitParams({})'.format(" ".join(self.as_flags()))
 
+    @deprecated.deprecated('3.7.0', '4.0.0',
+                           'Use fsl.wrappers.fsl_sub instead')
     def __call__(self, *command, **kwargs):
         """
         Submits the command to the cluster.
@@ -254,6 +262,10 @@ def submit(*command, **kwargs):
     """
     Submits a given command to the cluster
 
+    The ``submit`` function is deprecated - you should use
+    :mod:`fsl.wrappers.fsl_sub` instead, or use the ``fsl_sub`` Python
+    library, which is available in FSL 6.0.5 and newer.
+
     You can pass the command and arguments as a single string, or as a regular or unpacked sequence.
 
     :arg command:        string or regular/unpacked sequence of strings with the job command
@@ -288,9 +300,14 @@ def submit(*command, **kwargs):
     return SubmitParams(**kwargs)(*command)
 
 
+@deprecated.deprecated('3.7.0', '4.0.0', 'Use fsl_sub.report instead')
 def info(job_ids) -> Dict[str, Optional[Dict[str, str]]]:
     """Gets information on a given job id
 
+    The ``info`` function is deprecated - you should use the
+    ``fsl_sub.report`` function from the ``fsl_sub`` Python library, which
+    is available in FSL 6.0.5 and newer.
+
     Uses `qstat -j <job_ids>`
 
     :arg job_ids: string with job id or (nested) sequence with jobs

fsl/utils/run.py

@@ -31,10 +31,10 @@ import os.path         as op
 import                    os
 
 from   fsl.utils.platform import platform as fslplatform
-import fsl.utils.fslsub                   as fslsub
 import fsl.utils.tempdir                  as tempdir
 import fsl.utils.path                     as fslpath
 
+
 log = logging.getLogger(__name__)
 
 
@@ -52,17 +52,16 @@ class FSLNotPresent(Exception):
     """Error raised by the :func:`runfsl` function when ``$FSLDIR`` cannot
     be found.
     """
-    pass
 
 
 @contextlib.contextmanager
-def dryrun(*args):
+def dryrun(*_):
     """Context manager which causes all calls to :func:`run` to be logged but
     not executed. See the :data:`DRY_RUN` flag.
 
     The returned standard output will be equal to ``' '.join(args)``.
     """
-    global DRY_RUN
+    global DRY_RUN  # pylint: disable=global-statement
 
     oldval  = DRY_RUN
     DRY_RUN = True
@@ -146,19 +145,21 @@ def run(*args, **kwargs):
 
     :arg submit:   Must be passed as a keyword argument. Defaults to ``None``.
                    If ``True``, the command is submitted as a cluster job via
-                   the :func:`.fslsub.submit` function.  May also be a
+                   the :mod:`fsl.wrappers.fsl_sub` function.  May also be a
                    dictionary containing arguments to that function.
 
     :arg cmdonly:  Defaults to ``False``. If ``True``, the command is not
                    executed, but rather is returned directly, as a list of
                    arguments.
 
-    :arg log:      Must be passed as a keyword argument.  An optional ``dict``
-                   which may be used to redirect the command's standard output
-                   and error. The following keys are recognised:
+    :arg log:      Must be passed as a keyword argument.  Defaults to
+                   ``{'tee' : True}``. An optional ``dict`` which may be used
+                   to redirect the command's standard output and error. The
+                   following keys are recognised:
 
-                     - tee:    If ``True``, the command's standard output/error
-                               streams are forwarded to this processes streams.
+                     - tee:    If ``True`` (the default), the command's
+                               standard output/error streams are forwarded to
+                               this processes streams.
 
                      - stdout: Optional file-like object to which the command's
                                standard output stream can be forwarded.
@@ -173,10 +174,10 @@ def run(*args, **kwargs):
     object (via :func:`_realrun`), unless ``submit=True``, in which case they
     are passed through to the :func:`.fslsub.submit` function.
 
-    :returns:      If ``submit`` is provided, the return value of
-                   :func:`.fslsub` is returned. Otherwise returns a single
-                   value or a tuple, based on the based on the ``stdout``,
-                   ``stderr``, and ``exitcode`` arguments.
+    :returns: If ``submit`` is provided, the ID of the submitted job is
+              returned as a string. Otherwise returns a single value or a
+              tuple, based on the based on the ``stdout``, ``stderr``, and
+              ``exitcode`` arguments.
     """
 
     returnStdout   = kwargs.pop('stdout',   True)
@@ -184,16 +185,16 @@ def run(*args, **kwargs):
     returnExitcode = kwargs.pop('exitcode', False)
     submit         = kwargs.pop('submit',   {})
     cmdonly        = kwargs.pop('cmdonly',  False)
-    log            = kwargs.pop('log',      None)
+    logg           = kwargs.pop('log',      None)
     args           = prepareArgs(args)
 
-    if log is None:
-        log = {}
+    if logg is None:
+        logg = {}
 
-    tee       = log.get('tee',    False)
-    logStdout = log.get('stdout', None)
-    logStderr = log.get('stderr', None)
-    logCmd    = log.get('cmd',    None)
+    tee       = logg.get('tee',    True)
+    logStdout = logg.get('stdout', None)
+    logStderr = logg.get('stderr', None)
+    logCmd    = logg.get('cmd',    None)
 
     if not bool(submit):
         submit = None
@@ -217,9 +218,12 @@ def run(*args, **kwargs):
         return _dryrun(
             submit, returnStdout, returnStderr, returnExitcode, *args)
 
-    # submit - delegate to fslsub
+    # submit - delegate to fsl_sub. This will induce a nested
+    # call back to this run function, which is a bit confusing,
+    # but harmless, as we've popped the "submit" arg above.
     if submit is not None:
-        return fslsub.submit(' '.join(args), **submit, **kwargs)
+        from fsl.wrappers import fsl_sub  # pylint: disable=import-outside-toplevel  # noqa: E501
+        return fsl_sub(*args, **submit, **kwargs)[0].strip()
 
     # Run directly - delegate to _realrun
     stdout, stderr, exitcode = _realrun(
@@ -391,25 +395,32 @@ def runfsl(*args, **kwargs):
 
 
 def wslcmd(cmdpath, *args):
+    """Convert a command + arguments into an equivalent set of arguments that
+    will run the command under Windows Subsystem for Linux
+
+    :param cmdpath: Fully qualified path to the command. This is essentially
+                    a WSL path not a Windows one since FSLDIR is specified
+                    as a WSL path, however it may have backslashes as path
+                    separators due to previous use of ``os.path.join``
+
+    :param args:    Sequence of command arguments (the first of which is the
+                    unqualified command name)
+
+    :return: If ``cmdpath`` exists and is executable in WSL, return a
+             sequence of command arguments which when executed will run the
+             command in WSL. Windows paths in the argument list will be
+             converted to WSL paths. If ``cmdpath`` was not executable in
+             WSL, returns None
     """
-    Convert a command + arguments into an equivalent set of arguments that will run the command
-    under Windows Subsystem for Linux
-
-    :param cmdpath: Fully qualified path to the command. This is essentially a WSL path not a Windows
-                    one since FSLDIR is specified as a WSL path, however it may have backslashes
-                    as path separators due to previous use of ``os.path.join``
-    :param args: Sequence of command arguments (the first of which is the unqualified command name)
-
-    :return: If ``cmdpath`` exists and is executable in WSL, return a sequence of command arguments
-             which when executed will run the command in WSL. Windows paths in the argument list will
-             be converted to WSL paths. If ``cmdpath`` was not executable in WSL, returns None
-    """
-    # Check if command exists in WSL (remembering that the command path may include FSLDIR which
-    # is a Windows path)
+    # Check if command exists in WSL (remembering
+    # that the command path may include FSLDIR
+    # which is a Windows path)
     cmdpath = fslpath.wslpath(cmdpath)
-    _stdout, _stderr, retcode = _realrun(False, None, None, None, "wsl", "test", "-x", cmdpath)
+    _stdout, _stderr, retcode = _realrun(
+        False, None, None, None, "wsl", "test", "-x", cmdpath)
     if retcode == 0:
-        # Form a new argument list and convert any Windows paths in it into WSL paths
+        # Form a new argument list and convert
+        # any Windows paths in it into WSL paths
         wslargs = [fslpath.wslpath(arg) for arg in args]
         wslargs[0] = cmdpath
         local_fsldir = fslpath.wslpath(fslplatform.fsldir)
@@ -417,8 +428,10 @@ def wslcmd(cmdpath, *args):
             local_fsldevdir = fslpath.wslpath(fslplatform.fsldevdir)
         else:
             local_fsldevdir = None
-        # Prepend important environment variables - note that it seems we cannot
-        # use WSLENV for this due to its insistance on path mapping. FIXME FSLDEVDIR?
+        # Prepend important environment variables -
+        # note that it seems we cannot use WSLENV
+        # for this due to its insistance on path
+        # mapping. FIXME FSLDEVDIR?
         local_path = "$PATH"
         if local_fsldevdir:
             local_path += ":%s/bin" % local_fsldevdir
@@ -438,13 +451,17 @@ def wslcmd(cmdpath, *args):
 
 
 def hold(job_ids, hold_filename=None):
-    """
-    Waits until all jobs have finished
+    """Waits until all jobs have finished
+
+    :param job_ids: possibly nested sequence of job ids. The job ids
+                    themselves should be strings.
 
-    :param job_ids: possibly nested sequence of job ids. The job ids themselves should be strings.
-    :param hold_filename: filename to use as a hold file.
-        The containing directory should exist, but the file itself should not.
-        Defaults to a ./.<random characters>.hold in the current directory.
-    :return: only returns when all the jobs have finished
+    :param hold_filename: filename to use as a hold file.  The
+                          containing directory should exist, but the
+                          file itself should not.  Defaults to a
+                          ./.<random characters>.hold in the current
+                          directory.  :return: only returns when all
+                          the jobs have finished
     """
+    import fsl.utils.fslsub as fslsub  # pylint: disable=import-outside-toplevel  # noqa: E501
     fslsub.hold(job_ids, hold_filename)

fsl/wrappers/__init__.py

 #!/usr/bin/env python
 #
+# pylint: disable=unused-import
+# flake8: noqa: F401
+#
 # __init__.py - Wrappers for FSL command-line tools.
 #
 # Author: Paul McCarthy <pauldmccarthy@gmail.com>
@@ -79,36 +82,37 @@ decorators.
 """
 
 
-from .wrapperutils import (LOAD,)           # noqa
-from .bet          import (bet,             # noqa
-                           robustfov)
-from .eddy         import (eddy_cuda,       # noqa
-                           topup,
-                           applytopup)
-from .fast         import (fast,)           # noqa
-from .fsl_anat     import (fsl_anat,)       # noqa
-from .flirt        import (flirt,           # noqa
-                           invxfm,
-                           applyxfm,
-                           applyxfm4D,
-                           concatxfm,
-                           mcflirt)
-from .fnirt        import (fnirt,           # noqa
-                           applywarp,
-                           invwarp,
-                           convertwarp)
-from .fslmaths     import (fslmaths,)       # noqa
-from .fslstats     import (fslstats,)       # noqa
-from .fugue        import (fugue,           # noqa
-                           prelude,
-                           sigloss,
-                           fsl_prepare_fieldmap)
-from .melodic      import (melodic,         # noqa
-                           fsl_regfilt)
-from .misc         import (fslreorient2std, # noqa
-                           fslroi,
-                           slicer,
-                           cluster,
-                           gps)
-from .epi_reg      import  epi_reg
-from .             import  tbss             # noqa
+from fsl.wrappers.wrapperutils import (LOAD,)
+from fsl.wrappers.bet          import (bet,
+                                       robustfov)
+from fsl.wrappers.eddy         import (eddy_cuda,
+                                       topup,
+                                       applytopup)
+from fsl.wrappers.fast         import (fast,)
+from fsl.wrappers.fsl_anat     import (fsl_anat,)
+from fsl.wrappers.fsl_sub      import (fsl_sub,)
+from fsl.wrappers.flirt        import (flirt,
+                                       invxfm,
+                                       applyxfm,
+                                       applyxfm4D,
+                                       concatxfm,
+                                       mcflirt)
+from fsl.wrappers.fnirt        import (fnirt,
+                                       applywarp,
+                                       invwarp,
+                                       convertwarp)
+from fsl.wrappers.fslmaths     import (fslmaths,)
+from fsl.wrappers.fslstats     import (fslstats,)
+from fsl.wrappers.fugue        import (fugue,
+                                       prelude,
+                                       sigloss,
+                                       fsl_prepare_fieldmap)
+from fsl.wrappers.melodic      import (melodic,
+                                       fsl_regfilt)
+from fsl.wrappers.misc         import (fslreorient2std,
+                                       fslroi,
+                                       slicer,
+                                       cluster,
+                                       gps)
+from fsl.wrappers.epi_reg      import  epi_reg
+from fsl.wrappers              import  tbss

fsl/wrappers/fsl_sub.py

+#!/usr/bin/env python
+#
+# fsl_sub.py - Wrapper for the fsl_sub command.
+#
+# Author: Paul McCarthy <pauldmccarthy@gmail.com>
+#
+"""This module provides the :func:`fsl_sub` function, a wrapper for the FSL
+`fsl_sub <https://git.fmrib.ox.ac.uk/fsl/fsl_sub>`_ command.
+"""
+
+
+from . import wrapperutils  as wutils
+
+
+@wutils.fslwrapper
+def fsl_sub(*args, **kwargs):
+    """Wrapper for the ``fsl_sub`` command.
+    """
+    cmd  = ['fsl_sub']
+    cmd += wutils.applyArgStyle('--', singlechar_args=True, **kwargs)
+    cmd += list(args)
+    return cmd

fsl/wrappers/wrapperutils.py

@@ -441,7 +441,7 @@ file should be loaded into memory and returned as a Python object.
 """
 
 
-class FileOrThing(object):
+class FileOrThing:
     """Decorator which ensures that certain arguments which are passed into the
     decorated function are always passed as file names. Both positional and
     keyword arguments can be specified.

tests/__init__.py

@@ -69,7 +69,7 @@ def touch(fname):
         pass
 
 
-class CaptureStdout(object):
+class CaptureStdout:
     """Context manager which captures stdout and stderr. """
 
     def __init__(self):

tests/test_run.py

@@ -119,12 +119,23 @@ def test_run_tee():
 
         capture = CaptureStdout()
 
+        # default behaviour is for tee=True
         with capture:
-            stdout = run.run('./script.sh 1 2 3', log={'tee' : True})
+            stdout = run.run('./script.sh 1 2 3')
+        assert stdout         == expstdout
+        assert capture.stdout == expstdout
 
+        with capture.reset():
+            stdout = run.run('./script.sh 1 2 3', log={'tee' : True})
         assert stdout         == expstdout
         assert capture.stdout == expstdout
 
+        # disable forwarding
+        with capture.reset():
+            stdout = run.run('./script.sh 1 2 3', log={'tee' : False})
+        assert stdout         == expstdout
+        assert capture.stdout == ''
+
         with capture.reset():
             stdout, stderr = run.run('./script.sh 1 2 3', stderr=True,
                                      log={'tee' : True})
@@ -268,9 +279,9 @@ def test_runfsl():
         run.FSL_PREFIX        = None
 
 
-def mock_submit(cmd, **kwargs):
-    if isinstance(cmd, str):
-        name = cmd.split()[0]
+def mock_fsl_sub(*cmd, **kwargs):
+    if len(cmd) == 1 and isinstance(cmd[0], str):
+        name = cmd[0].split()[0]
     else:
         name = cmd[0]
 
@@ -286,7 +297,7 @@ def mock_submit(cmd, **kwargs):
         for k in sorted(kwargs.keys()):
             f.write('{}: {}\n'.format(k, kwargs[k]))
 
-    return jid
+    return (jid, '')
 
 
 def test_run_submit():
@@ -304,7 +315,7 @@ def test_run_submit():
 
     with tempdir.tempdir(), \
          mockFSLDIR(), \
-         mock.patch('fsl.utils.fslsub.submit', mock_submit):
+         mock.patch('fsl.wrappers.fsl_sub', mock_fsl_sub):