Repository: natcap/invest
Pull request #505 (2021-02-26 19:01:25)

CV tests sensitive to GEOS changes
This PR un-skips some CV tests now that geos has released a patch for a bug that was failing those tests. And our CV regression data is updated to accommodate other geos changes that happened to change the order that CV point features were created. This does not impact the geometry or data, just the order of the features in the layer.

Here's the test data PR: https://bitbucket.org/natcap/invest-test-data/pull-requests/20/updated-regression-data-for-the-complete

Addresses #476 - we should leave this open though, because the geos bugfix did not resolve the failing wave energy tests mentioned in that issue.

# Checklist
- [ ] Updated HISTORY.rst (if these changes are user-facing)

- [ ] Updated the user's guide (if needed)



Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index 79c480eb4..a0be0802b 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -52,8 +52,6 @@ Unreleased Changes (3.9.1)
     * Fixed an issue on Mac OS when certain models would loop indefinitely and
       never complete.  This was addressed by bumping the ``taskgraph``
       requirement version to ``0.10.3``
-    * Fixed a bug where saving a datastack parameter set with relative paths
-      would not convert Windows separators to linux style.
     * Provide a better validation error message when an overview '.ovr' file
       is input instead of a valid raster.
 * Carbon
diff --git a/Makefile b/Makefile
index ca9d0c82a..6c836b024 100644
--- a/Makefile
+++ b/Makefile
@@ -6,7 +6,7 @@ GIT_SAMPLE_DATA_REPO_REV    := b7a51f189315e08484b5ba997a5c1de88ab7f06d
 
 GIT_TEST_DATA_REPO          := https://bitbucket.org/natcap/invest-test-data.git
 GIT_TEST_DATA_REPO_PATH     := $(DATA_DIR)/invest-test-data
-GIT_TEST_DATA_REPO_REV      := 6fd5fa39cd9d81080caa7581f9acca7b9fadb7c8
+GIT_TEST_DATA_REPO_REV      := 69ea5fc56906c029bc745e5531b7a4d4e4c13237
 
 GIT_UG_REPO                  := https://github.com/natcap/invest.users-guide
 GIT_UG_REPO_PATH             := doc/users-guide
diff --git a/src/natcap/invest/datastack.py b/src/natcap/invest/datastack.py
index 99f036e6b..b594f0ac5 100644
--- a/src/natcap/invest/datastack.py
+++ b/src/natcap/invest/datastack.py
@@ -438,10 +438,10 @@ def extract_datastack_archive(datastack_path, dest_dir_path):
 
             # Archives always store linux-style paths.  os.path.normpath
             # converts forward slashes to backslashes if we're on Windows.
-            data_path = os.path.normpath(
-                os.path.join(dest_dir_path, args_param))
+            data_path = os.path.normpath(os.path.join(dest_dir_path,
+                                                      args_param))
             if os.path.exists(data_path):
-                return data_path
+                return os.path.normpath(data_path)
         return args_param
 
     new_args = _rewrite_paths(arguments_dict)
@@ -472,24 +472,19 @@ def build_parameter_set(args, model_name, paramset_path, relative=False):
         elif isinstance(args_param, list):
             return [_recurse(param) for param in args_param]
         elif isinstance(args_param, str):
-            # If args_param is empty string '' os.path.exists will be False
-            if os.path.exists(args_param):
-                normalized_path = os.path.normpath(args_param)
+            possible_path = args_param.replace('\\', '/')
+            if os.path.exists(possible_path):
+                # Always save unix paths.
+                normalized_path = os.path.normpath(possible_path)
                 if relative:
                     # Handle special case where python assumes that '.'
                     # represents the CWD
                     if (normalized_path == '.' or
                             os.path.dirname(paramset_path) == normalized_path):
                         return '.'
-                    temp_rel_path = os.path.relpath(
-                        normalized_path, os.path.dirname(paramset_path))
-                    # Always save unix paths.
-                    linux_style_path = temp_rel_path.replace('\\', '/')
-                else:
-                    # Always save unix paths.
-                    linux_style_path = normalized_path.replace('\\', '/')
-
-                return linux_style_path
+                    return os.path.relpath(normalized_path,
+                                           os.path.dirname(paramset_path))
+                return normalized_path
         return args_param
     parameter_data = {
         'model_name': model_name,


Test patch:
diff --git a/tests/test_coastal_vulnerability.py b/tests/test_coastal_vulnerability.py
index 2673aef76..f0c90f4e1 100644
--- a/tests/test_coastal_vulnerability.py
+++ b/tests/test_coastal_vulnerability.py
@@ -929,7 +929,6 @@ class CoastalVulnerabilityTests(unittest.TestCase):
                 if 'Must be a polyline vector' in err_strings:
                     raise ValueError(err_list)
 
-    @unittest.skip("Skipping for GEOS 3.9.0 bug.")
     def test_shore_points_on_single_polygon(self):
         """CV: test shore point creation with single polygon landmass."""
         aoi_path = os.path.join(self.workspace_dir, 'aoi.geojson')
@@ -1208,7 +1207,6 @@ class CoastalVulnerabilityTests(unittest.TestCase):
         layer = None
         vector = None
 
-    @unittest.skip("Skipping for GEOS 3.9.0 bug.")
     def test_prepare_landmass_invalid_geometry(self):
         """CV: test handling invalid geometries in landmass vector."""
         aoi_path = os.path.join(self.workspace_dir, 'aoi.geojson')
diff --git a/tests/test_datastack.py b/tests/test_datastack.py
index 21f1e319f..4c20df1e8 100644
--- a/tests/test_datastack.py
+++ b/tests/test_datastack.py
@@ -1,6 +1,5 @@
 """Testing Module for Datastack."""
 import os
-import sys
 import unittest
 import tempfile
 import shutil
@@ -481,49 +480,6 @@ class DatastacksTest(unittest.TestCase):
         self.assertEqual(invest_version, __version__)
         self.assertEqual(callable_name, modelname)
 
-    @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows")
-    def test_relative_parameter_set_windows(self):
-        """Datastack: test relative parameter set paths saved linux style."""
-        from natcap.invest import datastack, __version__
-
-        params = {
-            'foo': os.path.join(self.workspace, 'foo.txt'),
-            'bar': os.path.join(self.workspace, 'inter_dir', 'bar.txt'),
-            'doh': os.path.join(
-                self.workspace, 'inter_dir', 'inter_inter_dir', 'doh.txt'),
-            'data_dir': os.path.join(self.workspace, 'data_dir'),
-        }
-        os.makedirs(
-            os.path.join(self.workspace, 'inter_dir', 'inter_inter_dir'))
-        modelname = 'natcap.invest.foo'
-        paramset_filename = os.path.join(self.workspace, 'paramset.json')
-
-        # make the sample data so filepaths are interpreted correctly
-        for base_name in ('foo', 'bar', 'doh'):
-            open(params[base_name], 'w').write('hello!')
-        os.makedirs(params['data_dir'])
-
-        # Write the parameter set
-        datastack.build_parameter_set(
-            params, modelname, paramset_filename, relative=True)
-
-        # Check that the written parameter set file contains relative paths
-        raw_args = json.load(open(paramset_filename))['args']
-        self.assertEqual(raw_args['foo'], 'foo.txt')
-        # Expecting linux style path separators for Windows
-        self.assertEqual(raw_args['bar'], 'inter_dir/bar.txt')
-        self.assertEqual(raw_args['doh'], 'inter_dir/inter_inter_dir/doh.txt')
-        self.assertEqual(raw_args['data_dir'], 'data_dir')
-
-        # Read back the parameter set and verify the returned paths are
-        # absolute
-        args, callable_name, invest_version = datastack.extract_parameter_set(
-            paramset_filename)
-
-        self.assertEqual(args, params)
-        self.assertEqual(invest_version, __version__)
-        self.assertEqual(callable_name, modelname)
-
     def test_extract_parameters_from_logfile(self):
         """Datastacks: Verify we can read args from a logfile."""
         from natcap.invest import datastack
@@ -586,7 +542,7 @@ class DatastacksTest(unittest.TestCase):
         self.assertEqual(stack_info, datastack.ParameterSet(
             params, 'sample_model', natcap.invest.__version__))
 
-    def test_get_datastack_info_parameter_set(self):
+    def test_get_datatack_info_parameter_set(self):
         """Datastack: test get datastack info parameter set."""
         import natcap.invest
         from natcap.invest import datastack
@@ -604,9 +560,7 @@ class DatastacksTest(unittest.TestCase):
         stack_type, stack_info = datastack.get_datastack_info(json_path)
         self.assertEqual(stack_type, 'json')
         self.assertEqual(stack_info, datastack.ParameterSet(
-            params, 'sample_model', natcap.invest.__version__),
-            f"stack_info: {stack_info} v. datastack.ParameterSet : "
-            f"{datastack.ParameterSet(params, 'sample_model', natcap.invest.__version__)}")
+            params, 'sample_model', natcap.invest.__version__))
 
     def test_get_datastack_info_logfile_new_style(self):
         """Datastack: test get datastack info logfile new style."""
@@ -657,16 +611,15 @@ class DatastacksTest(unittest.TestCase):
         self.assertEqual(stack_info, datastack.ParameterSet(
             expected_args, datastack.UNKNOWN, datastack.UNKNOWN))
 
-    @unittest.skipUnless(sys.platform.startswith("win"), "requires Windows")
-    def test_mixed_path_separators_in_paramset_windows(self):
+    def test_mixed_path_separators_in_paramset(self):
         """Datastacks: parameter sets must handle windows and linux paths."""
         from natcap.invest import datastack
 
         args = {
-            'windows_path': os.path.join(
-                self.workspace, 'dir1\\filepath1.txt'),
-            'linux_path': os.path.join(
-                self.workspace, 'dir2/filepath2.txt'),
+            'windows_path': os.path.join(self.workspace,
+                                         'dir1\\filepath1.txt'),
+            'linux_path': os.path.join(self.workspace,
+                                       'dir2/filepath2.txt'),
         }
         for filepath in args.values():
             normalized_path = os.path.normpath(filepath.replace('\\', os.sep))
@@ -679,66 +632,22 @@ class DatastacksTest(unittest.TestCase):
                 open_file.write('the contents of this file do not matter.')
 
         paramset_path = os.path.join(self.workspace, 'paramset.invest.json')
-        # Windows paths should be saved with linux-style separators
-        datastack.build_parameter_set(
-            args, 'sample_model', paramset_path, relative=True)
-
-        with open(paramset_path) as saved_parameters:
-            args = json.loads(saved_parameters.read())['args']
-            # Expecting window_path to have linux style line seps
-            expected_args = {
-                'windows_path': 'dir1/filepath1.txt',
-                'linux_path': 'dir2/filepath2.txt',
-            }
-            self.assertEqual(expected_args, args)
-
-        expected_args = {
-            'windows_path': os.path.join(
-                self.workspace, 'dir1', 'filepath1.txt'),
-            'linux_path': os.path.join(
-                self.workspace, 'dir2', 'filepath2.txt'),
-        }
-
-        extracted_paramset = datastack.extract_parameter_set(paramset_path)
-        self.assertEqual(extracted_paramset.args, expected_args)
-
-    @unittest.skipUnless(sys.platform.startswith("darwin"), "requires macOS")
-    def test_mixed_path_separators_in_paramset_mac(self):
-        """Datastacks: parameter sets must handle mac and linux paths."""
-        from natcap.invest import datastack
-
-        args = {
-            'mac_path': os.path.join(
-                self.workspace, 'dir1/filepath1.txt'),
-            'linux_path': os.path.join(
-                self.workspace, 'dir2/filepath2.txt'),
-        }
-        for filepath in args.values():
-            try:
-                os.makedirs(os.path.dirname(filepath))
-            except OSError:
-                pass
-
-            with open(filepath, 'w') as open_file:
-                open_file.write('the contents of this file do not matter.')
-
-        paramset_path = os.path.join(self.workspace, 'paramset.invest.json')
-        datastack.build_parameter_set(
-            args, 'sample_model', paramset_path, relative=True)
+        datastack.build_parameter_set(args, 'sample_model', paramset_path,
+                                      relative=True)
 
         with open(paramset_path) as saved_parameters:
             args = json.loads(saved_parameters.read())['args']
             expected_args = {
-                'mac_path': 'dir1/filepath1.txt',
-                'linux_path': 'dir2/filepath2.txt',
+                'windows_path': os.path.join('dir1', 'filepath1.txt'),
+                'linux_path': os.path.join('dir2', 'filepath2.txt'),
             }
             self.assertEqual(expected_args, args)
 
         expected_args = {
-            'mac_path': os.path.join(
-                self.workspace, 'dir1', 'filepath1.txt'),
-            'linux_path': os.path.join(
-                self.workspace, 'dir2', 'filepath2.txt'),
+            'windows_path': os.path.join(self.workspace, 'dir1',
+                                         'filepath1.txt'),
+            'linux_path': os.path.join(self.workspace, 'dir2',
+                                       'filepath2.txt'),
         }
 
         extracted_paramset = datastack.extract_parameter_set(paramset_path)