From 0145cb25cf15de6c86979df76e83706273014022 Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Wed, 9 Feb 2022 13:35:32 -0700 Subject: [PATCH] Feature 966 Fix mask.poly logic (#1430) --- .../test_ensemble_stat_wrapper.py | 8 +-- .../point_stat/test_point_stat_wrapper.py | 20 ++++-- .../string_manip/test_util_string_manip.py | 6 ++ metplus/wrappers/command_builder.py | 47 -------------- metplus/wrappers/compare_gridded_wrapper.py | 3 - metplus/wrappers/ensemble_stat_wrapper.py | 16 ++--- metplus/wrappers/grid_diag_wrapper.py | 35 +++++++---- metplus/wrappers/grid_stat_wrapper.py | 18 ++++-- metplus/wrappers/mode_wrapper.py | 22 +++++-- metplus/wrappers/point_stat_wrapper.py | 62 +++++++------------ 10 files changed, 110 insertions(+), 127 deletions(-) diff --git a/internal_tests/pytests/ensemble_stat/test_ensemble_stat_wrapper.py b/internal_tests/pytests/ensemble_stat/test_ensemble_stat_wrapper.py index 3d5ac8896c..1c43f8d476 100644 --- a/internal_tests/pytests/ensemble_stat/test_ensemble_stat_wrapper.py +++ b/internal_tests/pytests/ensemble_stat/test_ensemble_stat_wrapper.py @@ -262,7 +262,7 @@ def test_handle_climo_file_variables(metplus_config, config_overrides, {'METPLUS_MASK_GRID': 'grid = ["FULL"];', 'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), # 13 mask grid and poly (new config var) ({'ENSEMBLE_STAT_MASK_GRID': 'FULL', @@ -271,7 +271,7 @@ def test_handle_climo_file_variables(metplus_config, config_overrides, {'METPLUS_MASK_GRID': 'grid = ["FULL"];', 'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), # 14 mask grid value ({'ENSEMBLE_STAT_MASK_GRID': 'FULL', @@ -289,13 +289,13 @@ def test_handle_climo_file_variables(metplus_config, config_overrides, ({'ENSEMBLE_STAT_VERIFICATION_MASK_TEMPLATE': 'one, two', }, {'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), # 27 mask poly (new config var) ({'ENSEMBLE_STAT_MASK_POLY': 'one, two', }, {'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), # output_prefix ({'ENSEMBLE_STAT_OUTPUT_PREFIX': 'my_output_prefix'}, diff --git a/internal_tests/pytests/point_stat/test_point_stat_wrapper.py b/internal_tests/pytests/point_stat/test_point_stat_wrapper.py index 1db7b58d11..49fbcba475 100755 --- a/internal_tests/pytests/point_stat/test_point_stat_wrapper.py +++ b/internal_tests/pytests/point_stat/test_point_stat_wrapper.py @@ -98,14 +98,14 @@ def test_met_dictionary_in_var_options(metplus_config): 'POINT_STAT_VERIFICATION_MASK_TEMPLATE': 'one, two', }, {'METPLUS_MASK_GRID': 'grid = ["FULL"];', - 'METPLUS_MASK_POLY': 'poly = ["one","two"];', + 'METPLUS_MASK_POLY': 'poly = ["one", "two"];', }), # mask grid and poly (new config var) ({'POINT_STAT_MASK_GRID': 'FULL', 'POINT_STAT_MASK_POLY': 'one, two', }, {'METPLUS_MASK_GRID': 'grid = ["FULL"];', - 'METPLUS_MASK_POLY': 'poly = ["one","two"];', + 'METPLUS_MASK_POLY': 'poly = ["one", "two"];', }), # mask grid value ({'POINT_STAT_MASK_GRID': 'FULL', @@ -113,6 +113,18 @@ def test_met_dictionary_in_var_options(metplus_config): {'METPLUS_MASK_GRID': 'grid = ["FULL"];', }), + # mask.poly complex example + ({'POINT_STAT_MASK_POLY': ('["{ENV[MET_BUILD_BASE]}/share/met/poly/CAR.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/GLF.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/NAO.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/SAO.poly" ];'), + }, + {'METPLUS_MASK_POLY': + 'poly = ["{ENV[MET_BUILD_BASE]}/share/met/poly/CAR.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/GLF.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/NAO.poly", ' + '"{ENV[MET_BUILD_BASE]}/share/met/poly/SAO.poly"];', + }), # mask grid empty string (should create empty list) ({'POINT_STAT_MASK_GRID': '', }, @@ -123,13 +135,13 @@ def test_met_dictionary_in_var_options(metplus_config): ({'POINT_STAT_VERIFICATION_MASK_TEMPLATE': 'one, two', }, {'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), # mask poly (new config var) ({'POINT_STAT_MASK_POLY': 'one, two', }, {'METPLUS_MASK_POLY': - 'poly = ["one","two"];', + 'poly = ["one", "two"];', }), ({'POINT_STAT_MASK_SID': 'one, two', 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 1c6f2dfc35..8678831f3c 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 @@ -55,6 +55,12 @@ def test_remove_quotes(before, after): # 10: multiples {}s with commas separating quotes within []s ('{name="UGRD"; level=["P850","P500","P250"];}, {name="UGRD"; level=["P750","P600"];}', ['{name="UGRD"; level=["P850","P500","P250"];}', '{name="UGRD"; level=["P750","P600"];}']), + # 11: list with square braces and ending semi-colon (MET format) + ('["{ENV[MET_BUILD_BASE]}/share/met/poly/CAR.poly", "{ENV[MET_BUILD_BASE]}/share/met/poly/GLF.poly"];', + ["{ENV[MET_BUILD_BASE]}/share/met/poly/CAR.poly", "{ENV[MET_BUILD_BASE]}/share/met/poly/GLF.poly"]), + # 12: list with square braces and ending semi-colon (MET format) no quotes + ('[MET_BASE/poly/LMV.poly];', + ["MET_BASE/poly/LMV.poly"]), ] ) def test_getlist(string_list, output_list): diff --git a/metplus/wrappers/command_builder.py b/metplus/wrappers/command_builder.py index 1c7afa23e4..7a3b499530 100755 --- a/metplus/wrappers/command_builder.py +++ b/metplus/wrappers/command_builder.py @@ -1051,19 +1051,6 @@ def find_and_check_output_file(self, time_info=None, 'to process') return False - def format_list_string(self, list_string): - """!Add quotation marks around each comma separated item in the string""" - strings = [] - for string in list_string.split(','): - string = string.strip().replace('\'', '\"') - if not string: - continue - if string[0] != '"' and string[-1] != '"': - string = f'"{string}"' - strings.append(string) - - return ','.join(strings) - def check_for_externals(self): self.check_for_gempak() @@ -1272,40 +1259,6 @@ def _handle_grib_pds_field_info(self, v_name, v_level, thresh): # add closing curly brace for prob= return f'{field} }}' - def read_mask_poly(self): - """! Read old or new config variables used to set mask.poly in MET - config files - - @returns value from config or empty string if neither variable - is set - """ - app = self.app_name.upper() - conf_value = self.config.getraw('config', f'{app}_MASK_POLY', '') - if not conf_value: - conf_value = ( - self.config.getraw('config', - f'{app}_VERIFICATION_MASK_TEMPLATE', - '') - ) - return conf_value - - def get_verification_mask(self, time_info): - """!If verification mask template is set in the config file, - use it to find the verification mask filename""" - template = self.c_dict.get('MASK_POLY_TEMPLATE') - if not template: - return - - filenames = do_string_sub(template, - **time_info) - mask_list_string = self.format_list_string(filenames) - self.c_dict['VERIFICATION_MASK'] = mask_list_string - if self.c_dict.get('MASK_POLY_IS_LIST', True): - mask_list_string = f'[{mask_list_string}]' - mask_fmt = f"poly = {mask_list_string};" - self.c_dict['MASK_POLY'] = mask_fmt - self.env_var_dict['METPLUS_MASK_POLY'] = mask_fmt - def get_command(self): """! Builds the command to run the MET application @rtype string diff --git a/metplus/wrappers/compare_gridded_wrapper.py b/metplus/wrappers/compare_gridded_wrapper.py index 89965513cb..85bdc0084e 100755 --- a/metplus/wrappers/compare_gridded_wrapper.py +++ b/metplus/wrappers/compare_gridded_wrapper.py @@ -171,9 +171,6 @@ def run_at_time_once(self, time_info): """ self.clear() - # get verification mask if available - self.get_verification_mask(time_info) - var_list = util.sub_var_list(self.c_dict['VAR_LIST_TEMP'], time_info) diff --git a/metplus/wrappers/ensemble_stat_wrapper.py b/metplus/wrappers/ensemble_stat_wrapper.py index 99680e5db3..1b424c2ab1 100755 --- a/metplus/wrappers/ensemble_stat_wrapper.py +++ b/metplus/wrappers/ensemble_stat_wrapper.py @@ -294,6 +294,15 @@ def create_c_dict(self): metplus_configs=['ENSEMBLE_STAT_MASK_GRID'], extra_args={'allow_empty': True}) + self.add_met_config(name='poly', + data_type='list', + env_var_name='METPLUS_MASK_POLY', + metplus_configs=['ENSEMBLE_STAT_MASK_POLY', + 'ENSEMBLE_STAT_POLY', + ('ENSEMBLE_STAT_' + 'VERIFICATION_MASK_TEMPLATE')], + extra_args={'allow_empty': True}) + self.add_met_config(name='ci_alpha', data_type='list', extra_args={'remove_quotes': True}) @@ -313,8 +322,6 @@ def create_c_dict(self): self.add_met_config_window('obs_window') self.handle_obs_window_legacy(c_dict) - c_dict['MASK_POLY_TEMPLATE'] = self.read_mask_poly() - self.add_met_config( name='obs_quality_inc', data_type='list', @@ -506,11 +513,6 @@ def set_environment_variables(self, time_info): self.add_env_var("OBS_WINDOW_END", str(self.c_dict['OBS_WINDOW_END'])) - # read output prefix at this step to ensure that - # CURRENT_[FCST/OBS]_[NAME/LEVEL] is substituted correctly - self.add_env_var('VERIF_MASK', - self.c_dict.get('VERIFICATION_MASK', '')) - # support old method of setting variables in MET config files self.add_env_var('ENS_THRESH', self.c_dict.get('ENS_THRESH')) diff --git a/metplus/wrappers/grid_diag_wrapper.py b/metplus/wrappers/grid_diag_wrapper.py index dcdd3501b8..fbfbe608a1 100755 --- a/metplus/wrappers/grid_diag_wrapper.py +++ b/metplus/wrappers/grid_diag_wrapper.py @@ -35,6 +35,14 @@ class GridDiagWrapper(RuntimeFreqWrapper): 'METPLUS_MASK_DICT', ] + # handle deprecated env vars used pre v4.0.0 + DEPRECATED_WRAPPER_ENV_VAR_KEYS = [ + 'DESC', + 'DATA_FIELD', + 'DATA_FILE_TYPE', + 'VERIF_MASK', + ] + def __init__(self, config, instance=None): self.app_name = "grid_diag" self.app_path = os.path.join(config.getdir('MET_BIN_DIR'), @@ -81,7 +89,18 @@ def create_c_dict(self): data_type='FCST', met_tool=self.app_name) - c_dict['MASK_POLY_TEMPLATE'] = self.read_mask_poly() + # handle setting VERIF_MASK for old wrapped MET config files + self.add_met_config(name='poly', + data_type='list', + env_var_name='METPLUS_VERIF_MASK', + metplus_configs=['GRID_DIAG_MASK_POLY', + 'GRID_DIAG_POLY', + ('GRID_DIAG_' + 'VERIFICATION_MASK_TEMPLATE')], + extra_args={'allow_empty': True}) + self.env_var_dict['VERIF_MASK'] = ( + self.env_var_dict.get('METPLUS_VERIF_MASK', '') + ) return c_dict @@ -94,8 +113,8 @@ def set_environment_variables(self, time_info): """ data_dict = self.format_met_config_dict(self.c_dict, 'data', - ['DATA_FILE_TYPE', - 'DATA_FIELD_FMT']) + ['DATA_FILE_TYPE', + 'DATA_FIELD_FMT']) self.env_var_dict['METPLUS_DATA_DICT'] = data_dict # support old method of setting MET config variables @@ -108,13 +127,6 @@ def set_environment_variables(self, time_info): self.add_env_var('DESC', self.env_var_dict.get('METPLUS_DESC', '')) - verif_mask = self.c_dict.get('VERIFICATION_MASK', '') - if verif_mask: - verif_mask = f'poly = {verif_mask};' - - self.add_env_var('VERIF_MASK', - verif_mask) - super().set_environment_variables(time_info) def get_command(self): @@ -174,9 +186,6 @@ def run_at_time_custom(self, time_info): if not self.set_data_field(time_info): return - # get verification mask if available - self.get_verification_mask(time_info) - # get other configurations for command self.set_command_line_arguments(time_info) diff --git a/metplus/wrappers/grid_stat_wrapper.py b/metplus/wrappers/grid_stat_wrapper.py index 86ea6dfbfa..ad7650c0f9 100755 --- a/metplus/wrappers/grid_stat_wrapper.py +++ b/metplus/wrappers/grid_stat_wrapper.py @@ -58,6 +58,7 @@ class GridStatWrapper(CompareGriddedWrapper): DEPRECATED_WRAPPER_ENV_VAR_KEYS = [ 'CLIMO_MEAN_FILE', 'CLIMO_STDEV_FILE', + 'VERIF_MASK', ] OUTPUT_FLAGS = ['fho', @@ -197,8 +198,18 @@ def create_c_dict(self): self.handle_mask(single_value=False) - # handle setting VERIFICATION_MASK for old wrapped MET config files - c_dict['MASK_POLY_TEMPLATE'] = self.read_mask_poly() + # handle setting VERIF_MASK for old wrapped MET config files + self.add_met_config(name='poly', + data_type='list', + env_var_name='METPLUS_MASK_POLY', + metplus_configs=['GRID_STAT_MASK_POLY', + 'GRID_STAT_POLY', + ('GRID_STAT_' + 'VERIFICATION_MASK_TEMPLATE')], + extra_args={'allow_empty': True}) + self.env_var_dict['VERIF_MASK'] = ( + self.get_env_var_value('METPLUS_MASK_POLY', item_type='list') + ) self.handle_climo_cdf_dict() @@ -286,7 +297,4 @@ def set_environment_variables(self, time_info): self.add_env_var('NEIGHBORHOOD_COV_THRESH', cov_thresh) - self.add_env_var('VERIF_MASK', - self.c_dict.get('VERIFICATION_MASK', '')) - super().set_environment_variables(time_info) diff --git a/metplus/wrappers/mode_wrapper.py b/metplus/wrappers/mode_wrapper.py index edd59aeb4c..af7458fc33 100755 --- a/metplus/wrappers/mode_wrapper.py +++ b/metplus/wrappers/mode_wrapper.py @@ -60,6 +60,11 @@ class MODEWrapper(CompareGriddedWrapper): 'METPLUS_CT_STATS_FLAG', ] + # handle deprecated env vars used pre v4.0.0 + DEPRECATED_WRAPPER_ENV_VAR_KEYS = [ + 'VERIF_MASK', + ] + WEIGHTS = { 'centroid_dist': 'float', 'boundary_dist': 'float', @@ -364,9 +369,18 @@ def create_c_dict(self): self.handle_mask(single_value=True, get_flags=True) - # handle setting VERIFICATION_MASK for old wrapped MET config files - c_dict['MASK_POLY_TEMPLATE'] = self.read_mask_poly() - c_dict['MASK_POLY_IS_LIST'] = False + # handle setting VERIF_MASK for old wrapped MET config files + self.add_met_config(name='poly', + data_type='list', + env_var_name='METPLUS_VERIF_MASK', + metplus_configs=['MODE_MASK_POLY', + 'MODE_POLY', + ('MODE_' + 'VERIFICATION_MASK_TEMPLATE')], + extra_args={'allow_empty': True}) + self.env_var_dict['VERIF_MASK'] = ( + self.get_env_var_value('METPLUS_VERIF_MASK').strip('[]') + ) return c_dict @@ -396,8 +410,6 @@ def set_environment_variables(self, time_info): self.add_env_var("OBS_MERGE_THRESH", self.c_dict["OBS_MERGE_THRESH"]) self.add_env_var("FCST_MERGE_FLAG", self.c_dict["FCST_MERGE_FLAG"]) self.add_env_var("OBS_MERGE_FLAG", self.c_dict["OBS_MERGE_FLAG"]) - self.add_env_var('VERIF_MASK', self.c_dict.get('VERIFICATION_MASK', - '""')) super().set_environment_variables(time_info) diff --git a/metplus/wrappers/point_stat_wrapper.py b/metplus/wrappers/point_stat_wrapper.py index 2ea826e33b..e5f54b9f6c 100755 --- a/metplus/wrappers/point_stat_wrapper.py +++ b/metplus/wrappers/point_stat_wrapper.py @@ -50,6 +50,10 @@ class PointStatWrapper(CompareGriddedWrapper): DEPRECATED_WRAPPER_ENV_VAR_KEYS = [ 'CLIMO_MEAN_FILE', 'CLIMO_STDEV_FILE', + 'POINT_STAT_POLY', + 'POINT_STAT_GRID', + 'POINT_STAT_STATION_ID', + 'POINT_STAT_MESSAGE_TYPE', ] OUTPUT_FLAGS = ['fho', @@ -156,7 +160,9 @@ def create_c_dict(self): data_type='list', env_var_name='METPLUS_MASK_POLY', metplus_configs=['POINT_STAT_MASK_POLY', - 'POINT_STAT_POLY'], + 'POINT_STAT_POLY', + ('POINT_STAT_' + 'VERIFICATION_MASK_TEMPLATE')], extra_args={'allow_empty': True}) self.add_met_config(name='sid', @@ -185,8 +191,6 @@ def create_c_dict(self): self.config.getraw('config', 'POINT_STAT_OBS_VALID_END', '') ) - c_dict['MASK_POLY_TEMPLATE'] = self.read_mask_poly() - c_dict['FCST_PROB_THRESH'] = ( self.config.getstr('config', 'FCST_POINT_STAT_PROB_THRESH', '==0.1') @@ -263,6 +267,22 @@ def create_c_dict(self): if not c_dict['OUTPUT_DIR']: self.log_error('Must set POINT_STAT_OUTPUT_DIR in config file') + # handle old method of setting env vars in MET config files + # pull out value after equals sign before the last semi-colon of + # each value. If not set, then set the value to an empty string + self.env_var_dict['POINT_STAT_POLY'] = ( + self.get_env_var_value('METPLUS_MASK_POLY', item_type='list') + ) + self.env_var_dict['POINT_STAT_GRID'] = ( + self.get_env_var_value('METPLUS_MASK_GRID', item_type='list') + ) + self.env_var_dict['POINT_STAT_STATION_ID'] = ( + self.get_env_var_value('METPLUS_MASK_SID', item_type='list') + ) + self.env_var_dict['POINT_STAT_MESSAGE_TYPE'] = ( + self.get_env_var_value('METPLUS_MESSAGE_TYPE', item_type='list') + ) + return c_dict def add_obs_valid_args(self, time_info): @@ -282,38 +302,6 @@ def set_environment_variables(self, time_info=None): to add each environment variable to run the """ - # handle old method of setting env vars in MET config files - # pull out value after equals sign before the last semi-colon of - # each value. If not set, then set the value to an empty string - point_stat_poly = self.get_env_var_value('METPLUS_MASK_POLY') - if not point_stat_poly: - point_stat_poly = '[]' - point_stat_grid = self.get_env_var_value('METPLUS_MASK_GRID') - if not point_stat_grid: - point_stat_grid = '[]' - point_stat_sid = self.get_env_var_value('METPLUS_MASK_SID') - if not point_stat_sid: - point_stat_sid = '[]' - - point_stat_message_type = ( - self.get_env_var_value('METPLUS_MESSAGE_TYPE') - ) - - if not point_stat_message_type: - point_stat_message_type = '[]' - - self.add_env_var('POINT_STAT_POLY', - point_stat_poly) - - self.add_env_var('POINT_STAT_GRID', - point_stat_grid) - - self.add_env_var('POINT_STAT_STATION_ID', - point_stat_sid) - - self.add_env_var('POINT_STAT_MESSAGE_TYPE', - point_stat_message_type) - # add old method of setting env vars self.add_env_var("FCST_FIELD", self.c_dict.get('FCST_FIELD', '')) @@ -326,8 +314,4 @@ def set_environment_variables(self, time_info=None): str(self.c_dict['OBS_WINDOW_BEGIN'])) self.add_env_var('OBS_WINDOW_END', str(self.c_dict['OBS_WINDOW_END'])) - # add additional env vars if they are specified - self.add_env_var('VERIF_MASK', - self.c_dict.get('VERIFICATION_MASK', '')) - super().set_environment_variables(time_info)