Let PyGMT work with the conda GMT package on Windows (#434)

This PR makes PyGMT work with the conda GMT package on Windows:

- Fix how pygmt loads libraries on Windows
- Separate out the libgmt loading tests 
- No longer need to install Ghostscript via chocolatey
- Fix two failing tests on Windows
- Two environmental variables are needed in Windows CI job:

   - GMT_LIBRARY_PATH: `C:\Miniconda\envs\testing\Library\bin`
   - GMT_SHAREDIR: `C:\Miniconda\envs\testing\Library\share\gmt`

Author: seisman

.azure-pipelines.yml

@@ -155,15 +155,18 @@ jobs:
 ########################################################################################
 - job:
   displayName: 'Windows'
-  condition: False
 
   pool:
     vmImage: 'vs2017-win2016'
 
   variables:
     CONDA_REQUIREMENTS: requirements.txt
     CONDA_REQUIREMENTS_DEV: requirements-dev.txt
-    CONDA_INSTALL_EXTRA: "codecov"
+    CONDA_INSTALL_EXTRA: "codecov gmt=6.0.0"
+    # ctypes.CDLL cannot find conda's libraries
+    GMT_LIBRARY_PATH: 'C:\Miniconda\envs\testing\Library\bin'
+    # Due to potential bug in GMT, we have to set GMT_SHAREDIR manually
+    GMT_SHAREDIR: 'C:\Miniconda\envs\testing\Library\share\gmt'
 
   strategy:
     matrix:
@@ -179,12 +182,6 @@ jobs:
 
   steps:
 
-  # Install ghostscript separately since there is no Windows conda-forge package for it.
-  - bash: |
-      set -x -e
-      choco install ghostscript
-    displayName: Install ghostscript via chocolatey
-
   - powershell: Write-Host "##vso[task.prependpath]$env:CONDA\Scripts"
     displayName: Add conda to PATH
 

pygmt/clib/loading.py

@@ -37,85 +37,48 @@ def load_libgmt(env=None):
         couldn't access the functions).
 
     """
-    libpath = get_clib_path(env)
-    try:
-        libgmt = ctypes.CDLL(libpath)
-        check_libgmt(libgmt)
-    except OSError as err:
-        msg = "\n".join(
-            [
-                "Error loading the GMT shared library '{}':".format(libpath),
-                "{}".format(str(err)),
-            ]
-        )
-        raise GMTCLibNotFoundError(msg)
-    return libgmt
-
-
-def get_clib_path(env):
-    """
-    Get the path to the libgmt shared library.
-
-    Determine the file name and extension and append to the path set by
-    ``GMT_LIBRARY_PATH``, if any.
-
-    Parameters
-    ----------
-    env : dict or None
-        A dictionary containing the environment variables. If ``None``, will
-        default to ``os.environ``.
-
-    Returns
-    -------
-    libpath : str
-        The path to the libgmt shared library.
-
-    """
-    libname = clib_name()
     if env is None:
         env = os.environ
-    if "GMT_LIBRARY_PATH" in env:
-        libpath = os.path.join(env["GMT_LIBRARY_PATH"], libname)
-    else:
-        libpath = libname
-    return libpath
+    libnames = clib_name(os_name=sys.platform)
+    libpath = env.get("GMT_LIBRARY_PATH", "")
+    error = True
+    for libname in libnames:
+        try:
+            libgmt = ctypes.CDLL(os.path.join(libpath, libname))
+            check_libgmt(libgmt)
+            error = False
+            break
+        except OSError as err:
+            error = err
+    if error:
+        raise GMTCLibNotFoundError(
+            "Error loading the GMT shared library '{}':".format(", ".join(libnames))
+        )
+    return libgmt
 
 
-def clib_name(os_name=None, is_64bit=None):
+def clib_name(os_name):
     """
     Return the name of GMT's shared library for the current OS.
 
     Parameters
     ----------
-    os_name : str or None
-        The operating system name as given by ``sys.platform``
-        (the default if None).
-    is_64bit : bool or None
-        Whether or not the OS is 64bit. Only used if the OS is ``win32``. If
-        None, will determine automatically.
+    os_name : str
+        The operating system name as given by ``sys.platform``.
 
     Returns
     -------
-    libname : str
-        The name of GMT's shared library.
+    libname : list of str
+        List of possible names of GMT's shared library.
 
     """
-    if os_name is None:
-        os_name = sys.platform
-
-    if is_64bit is None:
-        is_64bit = sys.maxsize > 2 ** 32
-
     if os_name.startswith("linux"):
-        libname = "libgmt.so"
+        libname = ["libgmt.so"]
     elif os_name == "darwin":
         # Darwin is macOS
-        libname = "libgmt.dylib"
+        libname = ["libgmt.dylib"]
     elif os_name == "win32":
-        if is_64bit:
-            libname = "gmt_w64.dll"
-        else:
-            libname = "gmt_w32.dll"
+        libname = ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
     else:
         raise GMTOSError('Operating system "{}" not supported.'.format(sys.platform))
     return libname

pygmt/tests/test_clib.py

@@ -14,12 +14,9 @@
 
 from .. import clib
 from ..clib.session import FAMILIES, VIAS
-from ..clib.loading import clib_name, load_libgmt, check_libgmt, get_clib_path
 from ..clib.conversion import dataarray_to_matrix
 from ..exceptions import (
     GMTCLibError,
-    GMTOSError,
-    GMTCLibNotFoundError,
     GMTCLibNoSessionError,
     GMTInvalidInput,
     GMTVersionError,
@@ -70,54 +67,6 @@ def mock_get_libgmt_func(name, argtypes=None, restype=None):
     setattr(session, "get_libgmt_func", get_libgmt_func)
 
 
-def test_load_libgmt():
-    "Test that loading libgmt works and doesn't crash."
-    load_libgmt()
-
-
-def test_load_libgmt_fail():
-    "Test that loading fails when given a bad library path."
-    env = {"GMT_LIBRARY_PATH": "not/a/real/path"}
-    with pytest.raises(GMTCLibNotFoundError):
-        load_libgmt(env=env)
-
-
-def test_get_clib_path():
-    "Test that the correct path is found when setting GMT_LIBRARY_PATH."
-    # Get the real path to the library first
-    with clib.Session() as lib:
-        libpath = lib.info["library path"]
-    libdir = os.path.dirname(libpath)
-    # Assign it to the environment variable but keep a backup value to restore
-    # later
-    env = {"GMT_LIBRARY_PATH": libdir}
-
-    # Check that the path is determined correctly
-    path_used = get_clib_path(env=env)
-    assert os.path.samefile(path_used, libpath)
-    assert os.path.dirname(path_used) == libdir
-
-    # Check that loading libgmt works
-    load_libgmt(env=env)
-
-
-def test_check_libgmt():
-    "Make sure check_libgmt fails when given a bogus library"
-    with pytest.raises(GMTCLibError):
-        check_libgmt(dict())
-
-
-def test_clib_name():
-    "Make sure we get the correct library name for different OS names"
-    for linux in ["linux", "linux2", "linux3"]:
-        assert clib_name(linux) == "libgmt.so"
-    assert clib_name("darwin") == "libgmt.dylib"
-    assert clib_name("win32", is_64bit=True) == "gmt_w64.dll"
-    assert clib_name("win32", is_64bit=False) == "gmt_w32.dll"
-    with pytest.raises(GMTOSError):
-        clib_name("meh")
-
-
 def test_getitem():
     "Test that I can get correct constants from the C lib"
     ses = clib.Session()

pygmt/tests/test_clib_loading.py

@@ -0,0 +1,35 @@
+"""
+Test the functions that load libgmt
+"""
+import pytest
+
+from ..clib.loading import clib_name, load_libgmt, check_libgmt
+from ..exceptions import GMTCLibError, GMTOSError, GMTCLibNotFoundError
+
+
+def test_check_libgmt():
+    "Make sure check_libgmt fails when given a bogus library"
+    with pytest.raises(GMTCLibError):
+        check_libgmt(dict())
+
+
+def test_load_libgmt():
+    "Test that loading libgmt works and doesn't crash."
+    check_libgmt(load_libgmt())
+
+
+def test_load_libgmt_fail():
+    "Test that loading fails when given a bad library path."
+    env = {"GMT_LIBRARY_PATH": "not/a/real/path"}
+    with pytest.raises(GMTCLibNotFoundError):
+        load_libgmt(env=env)
+
+
+def test_clib_name():
+    "Make sure we get the correct library name for different OS names"
+    for linux in ["linux", "linux2", "linux3"]:
+        assert clib_name(linux) == ["libgmt.so"]
+    assert clib_name("darwin") == ["libgmt.dylib"]
+    assert clib_name("win32") == ["gmt.dll", "gmt_w64.dll", "gmt_w32.dll"]
+    with pytest.raises(GMTOSError):
+        clib_name("meh")

pygmt/tests/test_surface.py

@@ -90,8 +90,8 @@ def test_surface_with_outfile_param():
         )
         assert output is None  # check that output is None since outfile is set
         assert os.path.exists(path=TEMP_GRID)  # check that outfile exists at path
-        grid = xr.open_dataarray(TEMP_GRID)
-        assert isinstance(grid, xr.DataArray)  # check that netcdf grid loaded properly
+        with xr.open_dataarray(TEMP_GRID) as grid:
+            assert isinstance(grid, xr.DataArray)  # ensure netcdf grid loads ok
     finally:
         os.remove(path=TEMP_GRID)
     return output
@@ -108,8 +108,8 @@ def test_surface_short_aliases():
         output = surface(data=data, I="5m", R=[245, 255, 20, 30], G=TEMP_GRID)
         assert output is None  # check that output is None since outfile is set
         assert os.path.exists(path=TEMP_GRID)  # check that outfile exists at path
-        grid = xr.open_dataarray(TEMP_GRID)
-        assert isinstance(grid, xr.DataArray)  # check that netcdf grid loaded properly
+        with xr.open_dataarray(TEMP_GRID) as grid:
+            assert isinstance(grid, xr.DataArray)  # ensure netcdf grid loads ok
     finally:
         os.remove(path=TEMP_GRID)
     return output