Merge branch 'rf/imglob-perf' into 'master'

MNT: Improve performance of im* scripts

See merge request fsl/fslpy!310

CHANGELOG.rst

@@ -2,8 +2,8 @@ This document contains the ``fslpy`` release history in reverse chronological
 order.
 
 
-3.7.0 (Under development)
--------------------------
+3.7.0 (Friday 20th August 2021)
+-------------------------------
 
 
 Added
@@ -17,6 +17,10 @@ Added
 Changed
 ^^^^^^^
 
+
+* Performance of the :mod:`.imglob`, :mod:`.imln`, :mod:`imtest`, :mod:`.imrm`
+  and :mod:`.remove_ext` scripts has been improved, by re-organising them to
+  avoid unnecessary and expensive imports such as ``numpy``.
 * 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

fsl/scripts/imglob.py

@@ -10,14 +10,9 @@ NIFTI/ANALYZE image files.
 
 
 import                   sys
-import                   warnings
+import                   glob
 import fsl.utils.path as fslpath
 
-# See atlasq.py for explanation
-with warnings.catch_warnings():
-    warnings.filterwarnings("ignore", category=FutureWarning)
-    import fsl.data.image as fslimage
-
 
 usage = """
 Usage: imglob [-extension/extensions] <list of names>
@@ -25,8 +20,17 @@ Usage: imglob [-extension/extensions] <list of names>
        -extensions for image list with full extensions
 """.strip()
 
-exts   = fslimage.ALLOWED_EXTENSIONS
-groups = fslimage.FILE_GROUPS
+
+# The lists below are defined in the
+# fsl.data.image class, but are duplicated
+# here for performance (to avoid import of
+# nibabel/numpy/etc).
+exts   = ['.nii.gz', '.nii', '.img', '.hdr', '.img.gz', '.hdr.gz']
+"""List of supported image file extensions. """
+
+
+groups = [('.hdr', '.img'), ('.hdr.gz', '.img.gz')]
+"""List of known image file groups (image/header file pairs). """
 
 
 def imglob(paths, output=None):
@@ -59,12 +63,27 @@ def imglob(paths, output=None):
 
     imgfiles = []
 
+    # Expand any wildcard paths if provided.
+    # Depending on the way that imglob is
+    # invoked, this may not get done by the
+    # calling shell.
+    expanded = []
+    for path in paths:
+        if any(c in path for c in '*?[]'):
+            expanded.extend(glob.glob(path))
+        else:
+            expanded.append(path)
+
+    paths = expanded
+
     # Build a list of all image files (both
     # hdr and img and otherwise) that match
     for path in paths:
         try:
-            path = fslimage.removeExt(path)
-            imgfiles.extend(fslimage.addExt(path, unambiguous=False))
+            path = fslpath.removeExt(path, allowedExts=exts)
+            imgfiles.extend(fslpath.addExt(path,
+                                           allowedExts=exts,
+                                           unambiguous=False))
         except fslpath.PathError:
             continue
 

fsl/scripts/imln.py

@@ -17,19 +17,22 @@ to NIFTI image files.
 import os.path        as op
 import                   os
 import                   sys
-import                   warnings
-
 import fsl.utils.path as fslpath
 
 
-# See atlasq.py for explanation
-with warnings.catch_warnings():
-    warnings.filterwarnings("ignore", category=FutureWarning)
-    import fsl.data.image as fslimage
-
+# The lists below are defined in the
+# fsl.data.image class, but are duplicated
+# here for performance (to avoid import of
+# nibabel/numpy/etc).
+exts = ['.nii.gz', '.nii',
+        '.img',    '.hdr',
+        '.img.gz', '.hdr.gz',
+        '.mnc',    '.mnc.gz']
+"""List of file extensions that are supported by ``imtest``.
+"""
 
-ALLOWED_EXTENSIONS = fslimage.ALLOWED_EXTENSIONS + ['.mnc', '.mnc.gz']
-"""List of file extensions that are supported by ``imln``. """
+groups = [('.hdr', '.img'), ('.hdr.gz', '.img.gz')]
+"""List of known image file groups (image/header file pairs). """
 
 
 usage = """
@@ -50,8 +53,8 @@ def main(argv=None):
         return 1
 
     target, linkbase = argv
-    target           = fslpath.removeExt(target,   ALLOWED_EXTENSIONS)
-    linkbase         = fslpath.removeExt(linkbase, ALLOWED_EXTENSIONS)
+    target           = fslpath.removeExt(target,   exts)
+    linkbase         = fslpath.removeExt(linkbase, exts)
 
     # Target must exist, so we can
     # infer the correct extension(s).
@@ -59,8 +62,8 @@ def main(argv=None):
     # (e.g. a.img without a.hdr).
     try:
         targets = fslpath.getFileGroup(target,
-                                       allowedExts=ALLOWED_EXTENSIONS,
-                                       fileGroups=fslimage.FILE_GROUPS,
+                                       allowedExts=exts,
+                                       fileGroups=groups,
                                        unambiguous=True)
     except Exception as e:
         print(f'Error: {e}')
@@ -70,7 +73,7 @@ def main(argv=None):
         if not op.exists(target):
             continue
 
-        ext  = fslpath.getExt(target, ALLOWED_EXTENSIONS)
+        ext  = fslpath.getExt(target, exts)
         link = f'{linkbase}{ext}'
 
         try:

fsl/scripts/imrm.py

@@ -13,22 +13,22 @@ import itertools      as it
 import os.path        as op
 import                   os
 import                   sys
-import                   warnings
-
 import fsl.utils.path as fslpath
 
-# See atlasq.py for explanation
-with warnings.catch_warnings():
-    warnings.filterwarnings("ignore", category=FutureWarning)
-    import fsl.data.image as fslimage
-
 
 usage = """Usage: imrm <list of image names to remove>
 NB: filenames can be basenames or not
 """.strip()
 
 
-ALLOWED_EXTENSIONS = fslimage.ALLOWED_EXTENSIONS + ['.mnc', '.mnc.gz']
+# This list is defined in the
+# fsl.data.image class, but are duplicated
+# here for performance (to avoid import of
+# nibabel/numpy/etc).
+exts = ['.nii.gz', '.nii',
+        '.img',    '.hdr',
+        '.img.gz', '.hdr.gz',
+        '.mnc',    '.mnc.gz']
 """List of file extensions that are removed by ``imrm``. """
 
 
@@ -42,9 +42,9 @@ def main(argv=None):
         print(usage)
         return 1
 
-    prefixes = [fslpath.removeExt(p, ALLOWED_EXTENSIONS) for p in argv]
+    prefixes = [fslpath.removeExt(p, exts) for p in argv]
 
-    for prefix, ext in it.product(prefixes, ALLOWED_EXTENSIONS):
+    for prefix, ext in it.product(prefixes, exts):
 
         path = f'{prefix}{ext}'
 

fsl/scripts/imtest.py

@@ -11,18 +11,22 @@ not, without having to know the file suffix (.nii, .nii.gz, etc).
 
 import os.path        as op
 import                   sys
-import                   warnings
-
 import fsl.utils.path as fslpath
 
-# See atlasq.py for explanation
-with warnings.catch_warnings():
-    warnings.filterwarnings("ignore", category=FutureWarning)
-    import fsl.data.image as fslimage
 
+# The lists below are defined in the
+# fsl.data.image class, but are duplicated
+# here for performance (to avoid import of
+# nibabel/numpy/etc).
+exts = ['.nii.gz', '.nii',
+        '.img',    '.hdr',
+        '.img.gz', '.hdr.gz',
+        '.mnc',    '.mnc.gz']
+"""List of file extensions that are supported by ``imtest``.
+"""
 
-ALLOWED_EXTENSIONS = fslimage.ALLOWED_EXTENSIONS + ['.mnc', '.mnc.gz']
-"""List of file extensions that are supported by ``imln``. """
+groups = [('.hdr', '.img'), ('.hdr.gz', '.img.gz')]
+"""List of known image file groups (image/header file pairs). """
 
 
 def main(argv=None):
@@ -38,7 +42,7 @@ def main(argv=None):
         print('0')
         return 0
 
-    path = fslpath.removeExt(argv[0], ALLOWED_EXTENSIONS)
+    path = fslpath.removeExt(argv[0], exts)
     path = op.realpath(path)
 
     # getFileGroup will raise an error
@@ -47,8 +51,8 @@ def main(argv=None):
     # image) does not exist
     try:
         fslpath.getFileGroup(path,
-                             allowedExts=ALLOWED_EXTENSIONS,
-                             fileGroups=fslimage.FILE_GROUPS,
+                             allowedExts=exts,
+                             fileGroups=groups,
                              unambiguous=True)
         print('1')
     except fslpath.PathError:

fsl/scripts/remove_ext.py

@@ -7,21 +7,21 @@
 
 
 import                   sys
-import warnings
-
 import fsl.utils.path as fslpath
 
-# See atlasq.py for explanation
-with warnings.catch_warnings():
-    warnings.filterwarnings("ignore", category=FutureWarning)
-    import fsl.data.image as fslimage
-
 
 usage = """Usage: remove_ext <list of image paths to remove extension from>
 """.strip()
 
 
-ALLOWED_EXTENSIONS = fslimage.ALLOWED_EXTENSIONS + ['.mnc', '.mnc.gz']
+# This list is defined in the
+# fsl.data.image class, but are duplicated
+# here for performance (to avoid import of
+# nibabel/numpy/etc).
+exts = ['.nii.gz', '.nii',
+        '.img',    '.hdr',
+        '.img.gz', '.hdr.gz',
+        '.mnc',    '.mnc.gz']
 """List of file extensions that are removed by ``remove_ext``. """
 
 
@@ -40,7 +40,7 @@ def main(argv=None):
     removed = []
 
     for path in argv:
-        removed.append(fslpath.removeExt(path, ALLOWED_EXTENSIONS))
+        removed.append(fslpath.removeExt(path, exts))
 
     print(' '.join(removed))
 

fsl/utils/path.py

@@ -37,8 +37,6 @@ import            re
 
 from typing import Sequence, Tuple, Union
 
-from fsl.utils.platform import platform
-
 
 PathLike = Union[str, pathlib.Path]
 
@@ -596,6 +594,7 @@ def winpath(path):
     This requires WSL2 which supports the ``\\wsl$\\`` network path.
     wslpath is assumed to be an absolute path.
     """
+    from fsl.utils.platform import platform  # pylint: disable=import-outside-toplevel  # noqa: E501
     if not platform.fslwsl:
         return path
     else:

fsl/utils/platform.py

@@ -111,26 +111,15 @@ class Platform(notifier.Notifier):
         self.WX_MAC_CARBON = WX_MAC_CARBON
         self.WX_GTK        = WX_GTK
 
-        self.__inSSHSession = False
-        self.__inVNCSession = False
+        # initialise fsldir - see fsldir.setter
+        self.fsldir = self.fsldir
+
+        # These are all initialised on first access
         self.__glVersion    = None
         self.__glRenderer   = None
         self.__glIsSoftware = None
         self.__fslVersion   = None
-
-        # initialise fsldir - see fsldir.setter
-        self.fsldir = self.fsldir
-
-        # Determine if a display is available. We do
-        # this once at init (instead of on-demand in
-        # the canHaveGui method) because calling the
-        # IsDisplayAvailable function will cause the
-        # application to steal focus under OSX!
-        try:
-            import wx
-            self.__canHaveGui = wx.App.IsDisplayAvailable()
-        except ImportError:
-            self.__canHaveGui = False
+        self.__canHaveGui   = None
 
         # If one of the SSH_/VNC environment
         # variables is set, then we're probably
@@ -177,7 +166,7 @@ class Platform(notifier.Notifier):
         the event loop is called periodically, and so is not always running.
         """
         try:
-            import wx
+            import wx  # pylint: disable=import-outside-toplevel
             app = wx.GetApp()
 
             # TODO Previously this conditional
@@ -216,6 +205,17 @@ class Platform(notifier.Notifier):
         'Equivalent functionality is available in fsleyes-widgets.')
     def canHaveGui(self):
         """``True`` if it is possible to create a GUI, ``False`` otherwise. """
+
+        # Determine if a display is available. Note that
+        # calling the IsDisplayAvailable function will
+        # cause the application to steal focus under OSX!
+        if self.__canHaveGui is None:
+            try:
+                import wx  # pylint: disable=import-outside-toplevel
+                self.__canHaveGui = wx.App.IsDisplayAvailable()
+            except ImportError:
+                self.__canHaveGui = False
+
         return self.__canHaveGui
 
 
@@ -261,13 +261,13 @@ class Platform(notifier.Notifier):
         if not self.canHaveGui:
             return WX_UNKNOWN
 
-        import wx
+        import wx  # pylint: disable=import-outside-toplevel
 
         pi = [t.lower() for t in wx.PlatformInfo]
 
-        if   any(['cocoa'  in p for p in pi]): plat = WX_MAC_COCOA
-        elif any(['carbon' in p for p in pi]): plat = WX_MAC_CARBON
-        elif any(['gtk'    in p for p in pi]): plat = WX_GTK
+        if   any('cocoa'  in p for p in pi): plat = WX_MAC_COCOA
+        elif any('carbon' in p for p in pi): plat = WX_MAC_CARBON
+        elif any('gtk'    in p for p in pi): plat = WX_GTK
         else:                                plat = WX_UNKNOWN
 
         if plat is WX_UNKNOWN:
@@ -290,7 +290,7 @@ class Platform(notifier.Notifier):
         if not self.canHaveGui:
             return WX_UNKNOWN
 
-        import wx
+        import wx  # pylint: disable=import-outside-toplevel
 
         pi        = [t.lower() for t in wx.PlatformInfo]
         isPhoenix = False
@@ -323,7 +323,9 @@ class Platform(notifier.Notifier):
 
     @property
     def fslwsl(self):
-        """Boolean flag indicating whether FSL is installed in Windows Subsystem for Linux """
+        """Boolean flag indicating whether FSL is installed in Windows
+        Subsystem for Linux
+        """
         return self.fsldir is not None and self.fsldir.startswith("\\\\wsl$")
 
 
@@ -352,8 +354,9 @@ class Platform(notifier.Notifier):
             if op.exists(versionFile):
                 with open(versionFile, 'rt') as f:
                     # split string at colon for new hash style versions
-                    # first object in list is the non-hashed version string (e.g. 6.0.2)
-                    # if no ":hash:" then standard FSL version string is still returned
+                    # first object in list is the non-hashed version string
+                    # (e.g. 6.0.2) if no ":hash:" then standard FSL version
+                    # string is still returned
                     self.__fslVersion = f.read().strip().split(":")[0]
 
         self.notify(value=value)

fsl/version.py

@@ -47,7 +47,7 @@ import            re
 import            string
 
 
-__version__ = '3.7.0.dev0'
+__version__ = '3.8.0.dev0'
 """Current version number, as a string. """
 
 

tests/test_scripts/test_imglob.py

@@ -100,6 +100,9 @@ def test_imglob_shouldPass2():
         ('file1.hdr file1.img file2.nii', 'file1 file2', 'primary', 'file1.hdr file2.nii'),
         ('file1.hdr file1.img file2.nii', 'file1 file2', 'all',     'file1.hdr file1.img file2.nii'),
 
+        # muiltiple files, given wildcard
+        ('file1.nii file2.nii', 'file*', 'prefix',  'file1 file2'),
+
         # no file
         ('file.nii', 'bag', 'prefix',  ''),
         ('file.nii', 'bag', 'primary', ''),