From f9404720c8db9be29dc22cd11a42e58deb201117 Mon Sep 17 00:00:00 2001
From: George McCabe <23407799+georgemccabe@users.noreply.github.com>
Date: Wed, 31 May 2023 15:34:29 -0600
Subject: [PATCH] Bugfix #2189 develop - spaces in complex thresholds (#2191)

---
 .../config_metplus/test_config_metplus.py     |  3 +-
 .../string_manip/test_util_string_manip.py    |  2 ++
 .../grid_stat/test_grid_stat_wrapper.py       |  6 ++--
 metplus/util/config_metplus.py                | 16 +++++-----
 metplus/util/string_manip.py                  | 32 ++++++++++---------
 5 files changed, 32 insertions(+), 27 deletions(-)

diff --git a/internal/tests/pytests/util/config_metplus/test_config_metplus.py b/internal/tests/pytests/util/config_metplus/test_config_metplus.py
index 5b374fe011..8dda66dea1 100644
--- a/internal/tests/pytests/util/config_metplus/test_config_metplus.py
+++ b/internal/tests/pytests/util/config_metplus/test_config_metplus.py
@@ -907,8 +907,7 @@ def test_getraw_instance_with_unset_var(metplus_config):
     ]
 )
 @pytest.mark.util
-def test_format_var_items_options_semicolon(config_value,
-                                            expected_result):
+def test_format_var_items_options_semicolon(config_value, expected_result):
     time_info = {}
 
     field_configs = {'name': 'FNAME',
diff --git a/internal/tests/pytests/util/string_manip/test_util_string_manip.py b/internal/tests/pytests/util/string_manip/test_util_string_manip.py
index 22453a53fe..d53f19b469 100644
--- a/internal/tests/pytests/util/string_manip/test_util_string_manip.py
+++ b/internal/tests/pytests/util/string_manip/test_util_string_manip.py
@@ -128,6 +128,8 @@ def test_threshold(key, value):
         ("goSFP90", None),
         ("NA", [('NA', '')]),
         ("<USP90(2.5)", [('<', 'USP90(2.5)')]),
+        ('gt4 && lt5', [('gt', 4), ('lt', 5)]),
+        (' gt4', [('gt', 4)]),
     ]
 )
 @pytest.mark.util
diff --git a/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py b/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py
index 0b1416a2ea..71d65503a8 100644
--- a/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py
+++ b/internal/tests/pytests/wrappers/grid_stat/test_grid_stat_wrapper.py
@@ -14,9 +14,10 @@
 obs_name = 'APCP_03'
 obs_level_no_quotes = '(*,*)'
 obs_level = f'"{obs_level_no_quotes}"'
-fcst_fmt = f'field = [{{ name="{fcst_name}"; level="{fcst_level}"; }}];'
+both_thresh = ' lt-0.5,gt-0.5 && lt0.5,gt0.5 '
+fcst_fmt = f'field = [{{ name="{fcst_name}"; level="{fcst_level}"; cat_thresh=[{both_thresh}]; }}];'
 obs_fmt = (f'field = [{{ name="{obs_name}"; '
-           f'level="{obs_level_no_quotes}"; }}];')
+           f'level="{obs_level_no_quotes}"; cat_thresh=[{both_thresh}]; }}];')
 time_fmt = '%Y%m%d%H'
 run_times = ['2005080700', '2005080712']
 
@@ -52,6 +53,7 @@ def set_minimum_config_settings(config):
     config.set('config', 'FCST_VAR1_LEVELS', fcst_level)
     config.set('config', 'OBS_VAR1_NAME', obs_name)
     config.set('config', 'OBS_VAR1_LEVELS', obs_level)
+    config.set('config', 'BOTH_VAR1_THRESH', both_thresh)
 
 
 @pytest.mark.parametrize(
diff --git a/metplus/util/config_metplus.py b/metplus/util/config_metplus.py
index 6d0fa4295b..61d0cad416 100644
--- a/metplus/util/config_metplus.py
+++ b/metplus/util/config_metplus.py
@@ -963,7 +963,8 @@ def parse_var_list(config, time_info=None, data_type=None, met_tool=None,
                                                        index,
                                                        search_prefixes)
 
-            field_info = _format_var_items(field_configs, time_info)
+            field_info = _format_var_items(field_configs, time_info,
+                                           config.logger)
             if not isinstance(field_info, dict):
                 config.logger.error(f'Could not process {current_type}_'
                                     f'VAR{index} variables: {field_info}')
@@ -1102,11 +1103,12 @@ def _find_var_name_indices(config, data_types, met_tool=None):
     return [int(index) for index in indices]
 
 
-def _format_var_items(field_configs, time_info=None):
+def _format_var_items(field_configs, time_info=None, logger=None):
     """! Substitute time information into field information and format values.
 
         @param field_configs dictionary with config variable names to read
         @param time_info dictionary containing time info for current run
+        @param logger (optional) logging object
         @returns dictionary containing name, levels, and output_names, as
          well as thresholds and extra options if found. If not enough
          information was set in the METplusConfig object, an empty
@@ -1128,16 +1130,14 @@ def _format_var_items(field_configs, time_info=None):
 
     # perform string substitution on name
     if time_info:
-        search_name = do_string_sub(search_name,
-                                    skip_missing_tags=True,
-                                    **time_info)
+        search_name = do_string_sub(search_name, **time_info,
+                                    skip_missing_tags=True)
     var_items['name'] = search_name
 
     # get levels, performing string substitution on each item of list
     for level in getlist(field_configs.get('levels')):
         if time_info:
-            level = do_string_sub(level,
-                                  **time_info)
+            level = do_string_sub(level, **time_info)
         var_items['levels'].append(level)
 
     # if no levels are found, add an empty string
@@ -1149,7 +1149,7 @@ def _format_var_items(field_configs, time_info=None):
     search_thresh = field_configs.get('thresh')
     if search_thresh:
         thresh = getlist(search_thresh)
-        if not validate_thresholds(thresh):
+        if not validate_thresholds(thresh, logger):
             return 'Invalid threshold supplied'
 
         var_items['thresh'] = thresh
diff --git a/metplus/util/string_manip.py b/metplus/util/string_manip.py
index da72ca9ca0..fd523c6ce2 100644
--- a/metplus/util/string_manip.py
+++ b/metplus/util/string_manip.py
@@ -311,20 +311,18 @@ def camel_to_underscore(camel):
 def get_threshold_via_regex(thresh_string):
     """!Ensure thresh values start with >,>=,==,!=,<,<=,gt,ge,eq,ne,lt,le and then a number
         Optionally can have multiple comparison/number pairs separated with && or ||.
-        Args:
-            @param thresh_string: String to examine, i.e. <=3.4
-        Returns:
-            None if string does not match any valid comparison operators or does
-              not contain a number afterwards
-            regex match object with comparison operator in group 1 and
-            number in group 2 if valid
+
+        @param thresh_string: String to examine, i.e. <=3.4
+        @returns None if string does not match any valid comparison operators
+         or does not contain a number afterwards. Regex match object with
+          comparison operator in group 1 and number in group 2 if valid
     """
 
     comparison_number_list = []
     # split thresh string by || or &&
     thresh_split = re.split(r'\|\||&&', thresh_string)
     # check each threshold for validity
-    for thresh in thresh_split:
+    for thresh in [item.strip() for item in thresh_split]:
         found_match = False
         for comp in list(VALID_COMPARISONS)+list(VALID_COMPARISONS.values()):
             # if valid, add to list of tuples
@@ -359,14 +357,14 @@ def get_threshold_via_regex(thresh_string):
     return comparison_number_list
 
 
-def validate_thresholds(thresh_list):
+def validate_thresholds(thresh_list, logger=None):
     """ Checks list of thresholds to ensure all of them have the correct format
         Should be a comparison operator with number pair combined with || or &&
         i.e. gt4 or >3&&<5 or gt3||lt1
-        Args:
-            @param thresh_list list of strings to check
-        Returns:
-            True if all items in the list are valid format, False if not
+
+        @param thresh_list list of strings to check
+        @param logger (optional) logging object to output error
+        @returns True if all items in the list are valid format, False if not
     """
     valid = True
     for thresh in thresh_list:
@@ -375,8 +373,12 @@ def validate_thresholds(thresh_list):
             valid = False
 
     if valid is False:
-        print("ERROR: Threshold values must use >,>=,==,!=,<,<=,gt,ge,eq,ne,lt, or le with a number, "
-              "optionally combined with && or ||")
+        err_str = ("Threshold values must use >,>=,==,!=,<,<=,gt,ge,eq,ne,lt, "
+                   "or le with a number, optionally combined with && or ||")
+        if logger:
+            logger.error(err_str)
+        else:
+            print(f'ERROR: {err_str}')
         return False
     return True