Merge pull request #1361 from emlys/task/334

Elevate warnings to errors in tests, except DeprecationWarnings

pyproject.toml

@@ -74,3 +74,7 @@ no_lines_before = 'LOCALFOLDER'
 
 [tool.setuptools.packages.find]
 where = ["src"]
+
+[tool.pytest.ini_options]
+# raise warnings to errors, except for deprecation warnings
+filterwarnings = ["error", "default::DeprecationWarning"]

src/natcap/invest/datastack.py

@@ -489,8 +489,9 @@ def extract_datastack_archive(datastack_path, dest_dir_path):
     _tarfile_safe_extract(datastack_path, dest_dir_path)
 
     # get the arguments dictionary
-    arguments_dict = json.load(open(
-        os.path.join(dest_dir_path, DATASTACK_PARAMETER_FILENAME)))['args']
+    with open(os.path.join(
+            dest_dir_path, DATASTACK_PARAMETER_FILENAME)) as datastack_file:
+        arguments_dict = json.load(datastack_file)['args']
 
     def _rewrite_paths(args_param):
         """Converts paths in `args_param` to paths in `dest_dir_path."""

src/natcap/invest/forest_carbon_edge_effect.py

@@ -861,8 +861,9 @@ def _calculate_tropical_forest_edge_carbon_map(
     #   kd_tree.data.shape: (d, 2)
     #   theta_model_parameters.shape: (d, 3)
     #   method_model_parameter.shape: (d,)
-    kd_tree, theta_model_parameters, method_model_parameter = pickle.load(
-        open(spatial_index_pickle_path, 'rb'))
+    with open(spatial_index_pickle_path, 'rb') as spatial_index_pickle_file:
+        kd_tree, theta_model_parameters, method_model_parameter = pickle.load(
+            spatial_index_pickle_file)
 
     # create output raster and open band for writing
     # fill nodata, in case we skip entire memory blocks that are non-forest

src/natcap/invest/ndr/ndr.py

@@ -1204,11 +1204,12 @@ def _normalize_raster(base_raster_path_band, target_normalized_raster_path):
     value_mean = value_sum
     if value_count > 0.0:
         value_mean /= value_count
+    target_nodata = float(numpy.finfo(numpy.float32).min)
 
     def _normalize_raster_op(array):
         """Divide values by mean."""
         result = numpy.empty(array.shape, dtype=numpy.float32)
-        result[:] = numpy.float32(base_nodata)
+        result[:] = target_nodata
 
         valid_mask = slice(None)
         if base_nodata is not None:
@@ -1218,11 +1219,6 @@ def _normalize_raster_op(array):
             result[valid_mask] /= value_mean
         return result
 
-    # It's possible for base_nodata to extend outside what can be represented
-    # in a float32, yet GDAL expects a python float.  Casting to numpy.float32
-    # and back to a python float allows for the nodata value to reflect the
-    # actual nodata pixel values.
-    target_nodata = float(numpy.float32(base_nodata))
     pygeoprocessing.raster_calculator(
         [base_raster_path_band], _normalize_raster_op,
         target_normalized_raster_path, gdal.GDT_Float32,

src/natcap/invest/recreation/out_of_core_quadtree.pyx

@@ -82,7 +82,8 @@ class OutOfCoreQuadTree(object):
         """Flush any cached data to disk."""
         self.node_data_manager.flush()
         if self.pickle_filename is not None:
-            pickle.dump(self, open(self.pickle_filename, 'wb'))
+            with open(self.pickle_filename, 'wb') as pickle_file:
+                pickle.dump(self, pickle_file)
 
     def build_node_shapes(self, ogr_polygon_layer):
         """Add features to an ogr.Layer to visualize quadtree segmentation.

src/natcap/invest/recreation/recmodel_client.py

@@ -660,7 +660,8 @@ def _retrieve_photo_user_days(
                 aoizip.write(filename, os.path.basename(filename))
 
     # convert shapefile to binary string for serialization
-    zip_file_binary = open(compressed_aoi_path, 'rb').read()
+    with open(compressed_aoi_path, 'rb') as aoifile:
+        zip_file_binary = aoifile.read()
 
     # transfer zipped file to server
     start_time = time.time()
@@ -674,8 +675,8 @@ def _retrieve_photo_user_days(
                 f'workspace_id: {workspace_id}')
 
     # unpack result
-    open(compressed_pud_path, 'wb').write(
-        result_zip_file_binary)
+    with open(compressed_pud_path, 'wb') as pud_file:
+        pud_file.write(result_zip_file_binary)
     temporary_output_dir = tempfile.mkdtemp(dir=output_dir)
     zipfile.ZipFile(compressed_pud_path, 'r').extractall(
         temporary_output_dir)

src/natcap/invest/recreation/recmodel_server.py

@@ -176,7 +176,8 @@ def fetch_workspace_aoi(self, workspace_id):  # pylint: disable=no-self-use
         workspace_path = os.path.join(self.cache_workspace, workspace_id)
         out_zip_file_path = os.path.join(
             workspace_path, str('server_in')+'.zip')
-        return open(out_zip_file_path, 'rb').read()
+        with open(out_zip_file_path, 'rb') as out_zipfile:
+            return out_zipfile.read()
 
     @_try_except_wrapper("exception in calc_photo_user_days_in_aoi")
     def calc_photo_user_days_in_aoi(
@@ -239,7 +240,8 @@ def calc_photo_user_days_in_aoi(
         LOGGER.info(
             'calc user days complete sending binary back on %s',
             workspace_path)
-        return open(aoi_pud_archive_path, 'rb').read(), workspace_id
+        with open(aoi_pud_archive_path, 'rb') as aoi_pud_archive:
+            return aoi_pud_archive.read(), workspace_id
 
     def _calc_aggregated_points_in_aoi(
             self, aoi_path, workspace_path, date_range, out_vector_filename):
@@ -268,7 +270,8 @@ def _calc_aggregated_points_in_aoi(
         pud_poly_feature_queue = multiprocessing.Queue(4)
         n_polytest_processes = multiprocessing.cpu_count()
 
-        global_qt = pickle.load(open(self.qt_pickle_filename, 'rb'))
+        with open(self.qt_pickle_filename, 'rb') as qt_pickle:
+            global_qt = pickle.load(qt_pickle)
         aoi_layer = aoi_vector.GetLayer()
         aoi_extent = aoi_layer.GetExtent()
         aoi_ref = aoi_layer.GetSpatialRef()
@@ -422,39 +425,39 @@ def _calc_aggregated_points_in_aoi(
         n_poly_tested = 0
 
         monthly_table_path = os.path.join(workspace_path, 'monthly_table.csv')
-        monthly_table = open(monthly_table_path, 'w')
         date_range_year = [
             date.tolist().timetuple().tm_year for date in date_range]
         table_headers = [
             '%s-%s' % (year, month) for year in range(
                 int(date_range_year[0]), int(date_range_year[1])+1)
             for month in range(1, 13)]
-        monthly_table.write('poly_id,' + ','.join(table_headers) + '\n')
+        with open(monthly_table_path, 'w') as monthly_table:
+            monthly_table.write('poly_id,' + ','.join(table_headers) + '\n')
 
-        while True:
-            result_tuple = pud_poly_feature_queue.get()
-            n_poly_tested += 1
-            if result_tuple == 'STOP':
-                n_processes_alive -= 1
-                if n_processes_alive == 0:
-                    break
-                continue
-            last_time = recmodel_client.delay_op(
-                last_time, LOGGER_TIME_DELAY, lambda: LOGGER.info(
-                    '%.2f%% of polygons tested', 100 * float(n_poly_tested) /
-                    pud_aoi_layer.GetFeatureCount()))
-            poly_id, pud_list, pud_monthly_set = result_tuple
-            poly_feat = pud_aoi_layer.GetFeature(poly_id)
-            for pud_index, pud_id in enumerate(pud_id_suffix_list):
-                poly_feat.SetField('PUD_%s' % pud_id, pud_list[pud_index])
-            pud_aoi_layer.SetFeature(poly_feat)
-
-            line = '%s,' % poly_id
-            line += (
-                ",".join(['%s' % len(pud_monthly_set[header])
-                          for header in table_headers]))
-            line += '\n'  # final newline
-            monthly_table.write(line)
+            while True:
+                result_tuple = pud_poly_feature_queue.get()
+                n_poly_tested += 1
+                if result_tuple == 'STOP':
+                    n_processes_alive -= 1
+                    if n_processes_alive == 0:
+                        break
+                    continue
+                last_time = recmodel_client.delay_op(
+                    last_time, LOGGER_TIME_DELAY, lambda: LOGGER.info(
+                        '%.2f%% of polygons tested', 100 * float(n_poly_tested) /
+                        pud_aoi_layer.GetFeatureCount()))
+                poly_id, pud_list, pud_monthly_set = result_tuple
+                poly_feat = pud_aoi_layer.GetFeature(poly_id)
+                for pud_index, pud_id in enumerate(pud_id_suffix_list):
+                    poly_feat.SetField('PUD_%s' % pud_id, pud_list[pud_index])
+                pud_aoi_layer.SetFeature(poly_feat)
+
+                line = '%s,' % poly_id
+                line += (
+                    ",".join(['%s' % len(pud_monthly_set[header])
+                              for header in table_headers]))
+                line += '\n'  # final newline
+                monthly_table.write(line)
 
         LOGGER.info('done with polygon test, syncing to disk')
         pud_aoi_layer = None
@@ -714,7 +717,8 @@ def _calc_poly_pud(
     """
     start_time = time.time()
     LOGGER.info('in a _calc_poly_process, loading %s', local_qt_pickle_path)
-    local_qt = pickle.load(open(local_qt_pickle_path, 'rb'))
+    with open(local_qt_pickle_path, 'rb') as qt_pickle:
+        local_qt = pickle.load(qt_pickle)
     LOGGER.info('local qt load took %.2fs', time.time() - start_time)
 
     aoi_vector = gdal.OpenEx(aoi_path, gdal.OF_VECTOR)

src/natcap/invest/recreation/recmodel_workspace_fetcher.py

@@ -50,7 +50,7 @@ def execute(args):
         args['workspace_id'])
 
     # unpack result
-    open(os.path.join(
-        output_dir, '%s.zip' % args['workspace_id']), 'wb').write(
-            workspace_aoi_binary)
+    with open(os.path.join(
+            output_dir, '%s.zip' % args['workspace_id']), 'wb') as file:
+        file.write(workspace_aoi_binary)
     LOGGER.info("fetched aoi")

src/natcap/invest/seasonal_water_yield/seasonal_water_yield.py

@@ -582,27 +582,6 @@ def execute(args):
     Returns:
         None.
     """
-    # This upgrades warnings to exceptions across this model.
-    # I found this useful to catch all kinds of weird inputs to the model
-    # during debugging and think it makes sense to have in production of this
-    # model too.
-    try:
-        warnings.filterwarnings('error')
-        _execute(args)
-    finally:
-        warnings.resetwarnings()
-
-
-def _execute(args):
-    """Execute the seasonal water yield model.
-
-    Args:
-        See the parameters for
-        `natcap.invest.seasonal_water_yield.seasonal_wateryield.execute`.
-
-    Returns:
-        None
-    """
     LOGGER.info('prepare and test inputs for common errors')
 
     # fail early on a missing required rain events table

tests/test_cli.py

@@ -44,7 +44,8 @@ def test_run_coastal_blue_carbon_workspace_in_json(self):
             os.path.dirname(__file__), '..', 'data', 'invest-test-data',
             'coastal_blue_carbon', 'cbc_galveston_bay.invs.json')
 
-        datastack_dict = json.load(open(parameter_set_path))
+        with open(parameter_set_path) as datastack_file:
+            datastack_dict = json.load(datastack_file)
         datastack_dict['args']['workspace_dir'] = self.workspace_dir
         new_parameter_set_path = os.path.join(
             self.workspace_dir, 'paramset.invs.json')
@@ -58,8 +59,7 @@ def test_run_coastal_blue_carbon_workspace_in_json(self):
             cli.main([
                 'run',
                 'coastal_blue_carbon',  # uses an exact modelname
-                '--datastack', new_parameter_set_path,
-                '--headless',  # unused, but recognized for backwards compat
+                '--datastack', new_parameter_set_path
             ])
         patched_model.assert_called_once()
 

tests/test_datastack.py

@@ -132,11 +132,11 @@ def test_collect_simple_parameters(self):
         self.assertEqual(len(os.listdir(out_directory)), 3)
 
         # We expect the workspace to be excluded from the resulting args dict.
-        self.assertEqual(
-            json.load(open(
-                os.path.join(out_directory,
-                             datastack.DATASTACK_PARAMETER_FILENAME)))['args'],
-            {'a': 1, 'b': 'hello there', 'c': 'plain bytestring', 'd': ''})
+        with open(os.path.join(
+                out_directory, datastack.DATASTACK_PARAMETER_FILENAME)) as file:
+            self.assertEqual(
+                json.load(file)['args'],
+                {'a': 1, 'b': 'hello there', 'c': 'plain bytestring', 'd': ''})
 
     def test_collect_rasters(self):
         """Datastack: test collect GDAL rasters."""
@@ -158,10 +158,10 @@ def test_collect_rasters(self):
             out_directory = os.path.join(self.workspace, 'extracted_archive')
             datastack._tarfile_safe_extract(archive_path, out_directory)
 
-            archived_params = json.load(
-                open(os.path.join(
+            with open(os.path.join(
                     out_directory,
-                    datastack.DATASTACK_PARAMETER_FILENAME)))['args']
+                    datastack.DATASTACK_PARAMETER_FILENAME)) as datastack_file:
+                archived_params = json.load(datastack_file)['args']
 
             self.assertEqual(len(archived_params), 1)
             model_array = pygeoprocessing.raster_to_numpy_array(
@@ -200,10 +200,10 @@ def test_collect_vectors(self):
             out_directory = os.path.join(dest_dir, 'extracted_archive')
             datastack._tarfile_safe_extract(archive_path, out_directory)
 
-            archived_params = json.load(
-                open(os.path.join(
+            with open(os.path.join(
                     out_directory,
-                    datastack.DATASTACK_PARAMETER_FILENAME)))['args']
+                    datastack.DATASTACK_PARAMETER_FILENAME)) as datastack_file:
+                archived_params = json.load(datastack_file)['args']
             _assert_vectors_equal(
                 params['vector'],
                 os.path.join(out_directory, archived_params['vector']))
@@ -242,9 +242,10 @@ def test_nonspatial_files(self):
         out_directory = os.path.join(self.workspace, 'extracted_archive')
         datastack._tarfile_safe_extract(archive_path, out_directory)
 
-        archived_params = json.load(
-            open(os.path.join(out_directory,
-                              datastack.DATASTACK_PARAMETER_FILENAME)))['args']
+        with open(os.path.join(
+                out_directory,
+                datastack.DATASTACK_PARAMETER_FILENAME)) as datastack_file:
+            archived_params = json.load(datastack_file)['args']
         self.assertTrue(filecmp.cmp(
             params['some_file'],
             os.path.join(out_directory, archived_params['some_file']),
@@ -279,9 +280,10 @@ def test_duplicate_filepaths(self):
         out_directory = os.path.join(self.workspace, 'extracted_archive')
         datastack._tarfile_safe_extract(archive_path, out_directory)
 
-        archived_params = json.load(
-            open(os.path.join(out_directory,
-                              datastack.DATASTACK_PARAMETER_FILENAME)))['args']
+        with open(os.path.join(
+                out_directory,
+                datastack.DATASTACK_PARAMETER_FILENAME)) as datastack_file:
+            archived_params = json.load(datastack_file)['args']
 
         # Assert that the archived 'foo' and 'bar' params point to the same
         # file.
@@ -479,15 +481,17 @@ def test_relative_parameter_set(self):
         # make the sample data so filepaths are interpreted correctly
         for file_base in ('foo', 'bar', 'file1', 'file2'):
             test_filepath = os.path.join(self.workspace, file_base + '.txt')
-            open(test_filepath, 'w').write('hello!')
+            with open(test_filepath, 'w') as file:
+                file.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']
+        with open(paramset_filename) as param_file:
+            raw_args = json.load(param_file)['args']
         self.assertEqual(raw_args['foo'], 'foo.txt')
         self.assertEqual(raw_args['bar'], 'foo.txt')
         self.assertEqual(raw_args['file_list'], ['file1.txt', 'file2.txt'])
@@ -523,15 +527,17 @@ def test_relative_parameter_set_windows(self):
 
         # make the sample data so filepaths are interpreted correctly
         for base_name in ('foo', 'bar', 'doh'):
-            open(params[base_name], 'w').write('hello!')
+            with open(params[base_name], 'w') as file:
+                file.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']
+        with open(paramset_filename) as param_file:
+            raw_args = json.load(param_file)['args']
         self.assertEqual(raw_args['foo'], 'foo.txt')
         # Expecting linux style path separators for Windows
         self.assertEqual(raw_args['bar'], 'inter_dir/bar.txt')