Repository: natcap/invest (python)

Issue:
HRA criteria table parsing throws cryptic error when section header is missing
Looking at a [user's forums post](https://community.naturalcapitalproject.org/t/hra-model-unboundlocalerror-local-variable-current-stressor-header-row/3424/2), the following exception was being raised:

```
>>> hra._parse_criteria_table('/Users/jdouglass/Downloads/2023-02-18-manipreyamrakash-hra-criteria-table/criteria_score.csv', 'target.csv')
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/Users/jdouglass/workspace/phargogh/invest/src/natcap/invest/hra.py", line 1781, in _parse_criteria_table
    attribute_name = table[current_stressor_header_row][col_index]
UnboundLocalError: local variable 'current_stressor_header_row' referenced before assignment
```

The problem is that the user's criteria table has the field `"HABITAT RESILIENCE ATIRIBUTES "` instead of `"HABITAT RESILIENCE ATTRIBUTES"`.  The typo is causing the program to break in a cryptic way.

Pull request description:
Improving criteria table parsing in HRA
This PR improves the usability of HRA's criteria table parsing by:

1. stripping all leading/trailing whitespace from string table cells (which should generally improve the likelihood of string matches between fields that must match)
2. explicitly checking that certain required section headers are found in the table.

The user's reported exception was due to number 2 above.  Number 1 became clear that it might be useful by looking at the user's table.

This PR depends on #1190 .

Fixes #1191 

## Checklist
- [x] Updated HISTORY.rst and link to any relevant issue (if these changes are user-facing)
- [ ] Updated the user's guide (if needed)
- [ ] Tested the affected models' UIs (if relevant)


Patch:
diff --git a/HISTORY.rst b/HISTORY.rst
index eb164fad3..dafa488c0 100644
--- a/HISTORY.rst
+++ b/HISTORY.rst
@@ -44,10 +44,20 @@ Unreleased Changes
 * Workbench
     * Added tooltips to the model tabs so that they can be identified even when
       several tabs are open (`#1071 <https://github.com/natcap/invest/issues/1088>`_)
+* HRA
+    * Fixed an issue where a cryptic exception was being thrown if the criteria
+      table's sections were not spelled exactly as expected.  There is now a
+      much more readable error if a section is obviously missing.  Leading and
+      trailing whitespace is also now removed from all string fields in the
+      criteria table, which should also help reduce the chance of errors.
+      https://github.com/natcap/invest/issues/1191
 * Scenic Quality
     * Any points over nodata (and therefore excluded from the viewshed
       analysis) will now correctly have their FID reported in the logging.
       https://github.com/natcap/invest/issues/1188
+    * Clarifying where the visual quality calculations' disk-based sorting
+      cache should be located, which addresses an interesting crash experienced
+      by some users on Windows. https://github.com/natcap/invest/issues/1189
 * SDR
     * The ``ws_id`` field is no longer a required field in the watershed vector.
       https://github.com/natcap/invest/issues/1201
diff --git a/src/natcap/invest/hra.py b/src/natcap/invest/hra.py
index 9dea92472..076311dc7 100644
--- a/src/natcap/invest/hra.py
+++ b/src/natcap/invest/hra.py
@@ -1703,12 +1703,32 @@ def _parse_criteria_table(criteria_table_path, target_composite_csv_path):
                              engine='python')
     table = df.to_numpy()
 
+    # clean up any leading or trailing whitespace.
+    for row_num in range(table.shape[0]):
+        for col_num in range(table.shape[1]):
+            value = table[row_num][col_num]
+            if isinstance(value, str):
+                table[row_num][col_num] = value.strip()
+
+    # Some fields are required and will lead to cryptic errors if not found or
+    # slightly misspelled.
+    habitat_name_header = 'HABITAT NAME'
+    overlap_section_header = 'HABITAT STRESSOR OVERLAP PROPERTIES'
+    habitat_resilience_header = 'HABITAT RESILIENCE ATTRIBUTES'
+    required_section_headers = {
+        habitat_name_header, overlap_section_header,
+        habitat_resilience_header}
+    missing_sections = required_section_headers - set(table[:, 0])
+    if missing_sections:
+        raise AssertionError(
+            "The criteria table is missing these section headers: "
+            f"{', '.join(missing_sections)}")
+
     # Habitats are loaded from the top row (table[0])
     known_habitats = set(table[0]).difference(
-        {'HABITAT NAME', numpy.nan, 'CRITERIA TYPE'})
+        {habitat_name_header, numpy.nan, 'CRITERIA TYPE'})
 
     # Stressors are loaded from the first column (table[:, 0])
-    overlap_section_header = 'HABITAT STRESSOR OVERLAP PROPERTIES'
     known_stressors = set()
     for row_index, value in enumerate(table[:, 0]):
         try:
@@ -1731,7 +1751,7 @@ def _parse_criteria_table(criteria_table_path, target_composite_csv_path):
     criteria_col = None
     habitats = set()
     for col_index, value in enumerate(table[0]):
-        if value == 'HABITAT NAME':
+        if value == habitat_name_header:
             continue
         if value == 'CRITERIA TYPE':
             criteria_col = col_index
@@ -1753,8 +1773,8 @@ def _parse_criteria_table(criteria_table_path, target_composite_csv_path):
             continue
 
         if (row[0] in known_stressors or
-                row[0] == 'HABITAT RESILIENCE ATTRIBUTES'):
-            if row[0] == 'HABITAT RESILIENCE ATTRIBUTES':
+                row[0] == habitat_resilience_header):
+            if row[0] == habitat_resilience_header:
                 row[0] = 'RESILIENCE'  # Shorten for convenience
             current_stressor = row[0]
             current_stressor_header_row = row_index
diff --git a/src/natcap/invest/scenic_quality/scenic_quality.py b/src/natcap/invest/scenic_quality/scenic_quality.py
index 52dd1f0dc..90241611e 100644
--- a/src/natcap/invest/scenic_quality/scenic_quality.py
+++ b/src/natcap/invest/scenic_quality/scenic_quality.py
@@ -981,8 +981,8 @@ def _calculate_visual_quality(source_raster_path, working_dir, target_path):
     raster_info = pygeoprocessing.get_raster_info(source_raster_path)
     raster_nodata = raster_info['nodata'][0]
 
-    temp_dir = tempfile.mkdtemp(dir=working_dir,
-                                prefix='visual_quality')
+    temp_dir = os.path.normpath(
+        tempfile.mkdtemp(dir=working_dir, prefix='visual_quality'))
 
     # phase 1: calculate percentiles from the visible_structures raster
     LOGGER.info('Determining percentiles for %s',
@@ -1006,8 +1006,11 @@ def _calculate_visual_quality(source_raster_path, working_dir, target_path):
         gdal.GDT_Float64, _VALUATION_NODATA,
         raster_driver_creation_tuple=FLOAT_GTIFF_CREATION_OPTIONS)
 
+    # The directory passed to raster_band_percentile() is expected to not exist
+    # or be completely empty.
+    percentiles_working_dir = os.path.join(temp_dir, 'percentiles_working_dir')
     percentile_values = pygeoprocessing.raster_band_percentile(
-        (masked_raster_path, 1), temp_dir, [0., 25., 50., 75.])
+        (masked_raster_path, 1), percentiles_working_dir, [0., 25., 50., 75.])
 
     shutil.rmtree(temp_dir, ignore_errors=True)
 


Test patch:
diff --git a/tests/test_hra.py b/tests/test_hra.py
index 30e5514a6..7ce7b3e9d 100644
--- a/tests/test_hra.py
+++ b/tests/test_hra.py
@@ -320,6 +320,40 @@ class HRAUnitTests(unittest.TestCase):
         self.assertIn("Criterion could not be opened as a spatial file",
                       str(cm.exception))
 
+    def test_criteria_table_missing_section_headers(self):
+        """HRA: verify exception when a required section is not found."""
+        from natcap.invest import hra
+
+        criteria_table_path = os.path.join(self.workspace_dir, 'criteria.csv')
+        with open(criteria_table_path, 'w') as criteria_table:
+            criteria_table.write(
+                textwrap.dedent(  # NOTE: also checking whitespace around
+                    """\
+                      HABITAT-NAME,eelgrass,,,hardbottom ,,,CRITERIA TYPE
+                    HABITAT-FOOOOO-ATTRIBUTES,RATING  ,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
+                    recruitment rate,2,2,2,2,2,2,C
+                    connectivity rate,foo/eelgrass_connectivity.shp,2,2,2,2,2,C
+                    ,,,,,,,
+                    HABITAT STRESSOR OVERLAP PROPERTIES,,,,,,,
+                    oil,RATING,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
+                    frequency of disturbance ,2,2,3,2,2,3,C
+                    management effectiveness,2,2,1,2,2,1,E
+                    ,,,,,,,
+                    fishing,RATING,DQ,WEIGHT,RATING,DQ,WEIGHT,E/C
+                    frequency of disturbance,2,2,3,2,2,3,C
+                    management effectiveness,2,2,1,2,2,1,E
+                    """
+                ))
+        target_composite_csv_path = os.path.join(self.workspace_dir,
+                                                 'composite.csv')
+        with self.assertRaises(AssertionError) as cm:
+            habitats, stressors = hra._parse_criteria_table(
+                criteria_table_path, target_composite_csv_path)
+        self.assertIn('The criteria table is missing these section headers',
+                      str(cm.exception))
+        self.assertIn('HABITAT NAME', str(cm.exception))
+        self.assertIn('HABITAT RESILIENCE ATTRIBUTES', str(cm.exception))
+
     def test_maximum_reclassified_score(self):
         """HRA: check maximum reclassed score given a stack of scores."""
         from natcap.invest import hra