Deprecate lib.util.which (#4340)

* deprecate lib.util.which

package/CHANGELOG

@@ -55,6 +55,8 @@ Changes
     `analysis.nucleicacids.WatsonCrickDist.results.distances` (Issue #3720, PR #3735)
 
 Deprecations
+  * MDAnalysis.lib.util is deprecated and will be removed in version 3.0
+    (Issue #3649)
   * The TRZ reader & writer are deprecated and will be removed in version 3.0
     (PR #4335)
   * X3DNA has been deprecated and will be removed in 3.0.0 (Issue #3788)

package/MDAnalysis/analysis/hole2/hole.py

@@ -22,6 +22,7 @@
 #
 import os
 import errno
+import shutil
 import tempfile
 import textwrap
 import logging
@@ -228,7 +229,7 @@ def hole(pdbfile,
         warnings.warn(msg.format(output_level))
 
     # get executable
-    exe = util.which(executable)
+    exe = shutil.which(executable)
     if exe is None:
         raise OSError(errno.ENOENT, exe_err.format(name=executable,
                                                    kw='executable'))
@@ -521,23 +522,23 @@ def __init__(self, universe,
         self.ignore_residues = ignore_residues
 
         # --- finding executables ----
-        hole = util.which(executable)
+        hole = shutil.which(executable)
         if hole is None:
             raise OSError(errno.ENOENT, exe_err.format(name=executable,
                                                        kw='executable'))
         self.base_path = os.path.dirname(hole)
 
-        sos_triangle_path = util.which(sos_triangle)
+        sos_triangle_path = shutil.which(sos_triangle)
         if sos_triangle_path is None:
             path = os.path.join(self.base_path, sos_triangle)
-            sos_triangle_path = util.which(path)
+            sos_triangle_path = shutil.which(path)
         if sos_triangle_path is None:
             raise OSError(errno.ENOENT, exe_err.format(name=sos_triangle,
                                                        kw='sos_triangle'))
-        sph_process_path = util.which(sph_process)
+        sph_process_path = shutil.which(sph_process)
         if sph_process_path is None:
             path = os.path.join(self.base_path, sph_process)
-            sph_process_path = util.which(path)
+            sph_process_path = shutil.which(path)
         if sph_process_path is None:
             raise OSError(errno.ENOENT, exe_err.format(name=sph_process,
                                                        kw='sph_process'))

package/MDAnalysis/lib/util.py

@@ -555,7 +555,16 @@ def which(program):
     path : str or None
        absolute path to the executable if it can be found, else ``None``
 
+
+    .. deprecated:: 2.7.0
+       This method is deprecated and will be removed in version 3.0.0.
+       Please use shutil.which instead.
     """
+    # Can't use decorator because it's declared after this method
+    wmsg = ("This method is deprecated as of MDAnalysis version 2.7.0 "
+            "and will be removed in version 3.0.0. Please use shutil.which "
+            "instead.")
+    warnings.warn(wmsg, DeprecationWarning)
 
     def is_exe(fpath):
         return os.path.isfile(fpath) and os.access(fpath, os.X_OK)

testsuite/MDAnalysisTests/lib/test_util.py

@@ -29,6 +29,7 @@
 from unittest.mock import Mock, patch
 import sys
 import copy
+import shutil
 
 import numpy as np
 from numpy.testing import (assert_equal, assert_almost_equal,
@@ -2222,3 +2223,12 @@ def test_store_arguments_withkwargs(self):
 
         store2 = store.copy()
         assert store2.__dict__ == store.__dict__
+
+
+@pytest.mark.xfail(os.name == 'nt',
+                   reason="util.which does not get right binary on Windows")
+def test_which():
+    wmsg = "This method is deprecated"
+
+    with pytest.warns(DeprecationWarning, match=wmsg):
+        assert util.which('python') == shutil.which('python')

testsuite/MDAnalysisTests/util.py

@@ -32,6 +32,7 @@
 from contextlib import contextmanager
 from functools import wraps
 import importlib
+import shutil
 from unittest import mock
 import os
 import warnings
@@ -81,11 +82,8 @@ def executable_not_found(*args):
 
     @dec.skipif(executable_not_found("binary_name"), msg="skip test because binary_name not available")
     """
-    # This must come here so that MDAnalysis isn't imported prematurely,
-    #  which spoils coverage accounting (see Issue 344).
-    import MDAnalysis.lib.util
     for name in args:
-        if MDAnalysis.lib.util.which(name) is not None:
+        if shutil.which(name) is not None:
             return False
     return True