From c1e94d493d06df880489ae8df001b131746a585b Mon Sep 17 00:00:00 2001 From: George McCabe <23407799+georgemccabe@users.noreply.github.com> Date: Thu, 19 Dec 2024 13:58:18 -0700 Subject: [PATCH] Feature #2586 GenVxMask improvements (#2833) * resolve some SonarQube complaints * per #2586, added function with tests to properly parse list of command line arguments that can now contain comma-separated lists that should not be split up into separate items * add support for {app}_{data_type}_FILE_WINDOW_BEGIN/END, e.g. GEN_VX_MASK_OBS_FILE_WINDOW_BEGIN. This just adds support for an additional variation of the config variable names * add support for an empty label for input templates * update wrapper to be consistent with other wrappers wrt finding input files, progress towards #2492. Allow file window range to be specified separately for mask and input files. Other cleanup to move towards consistent wrappers with fewer wrapper-specific overrides of functions like get_command * update unit tests to align with changes for #2492 * add documentation for config variables that are newly supported to allow file window range to be specified separately for mask and input files * renamed GEN_VX_MASK_OBS variables to be GEN_VX_MASK_INPUT as suggested by @JohnHalleyGotway in PR review * fix logic to properly read input files by handling inputs that support multiple inputs with labels (used by GridDiag and UserScript wrappers) and typical inputs (all other wrappers). Prior to this change only input templates that have the FCST or OBS identifier were read properly via get_input_templates --- docs/Users_Guide/glossary.rst | 50 ++++++- docs/Users_Guide/wrappers.rst | 4 + .../config_metplus/test_config_metplus.py | 85 ++--------- .../string_manip/test_util_string_manip.py | 19 +-- .../wrappers/gen_vx_mask/test_gen_vx_mask.py | 53 +++++-- metplus/util/string_manip.py | 51 +++---- metplus/wrappers/command_builder.py | 1 + metplus/wrappers/gen_vx_mask_wrapper.py | 132 +++++++++--------- metplus/wrappers/runtime_freq_wrapper.py | 16 ++- 9 files changed, 218 insertions(+), 193 deletions(-) diff --git a/docs/Users_Guide/glossary.rst b/docs/Users_Guide/glossary.rst index 94b11cc287..e370857fbd 100644 --- a/docs/Users_Guide/glossary.rst +++ b/docs/Users_Guide/glossary.rst @@ -4362,12 +4362,58 @@ METplus Configuration Glossary | *Used by:* GenVxMask GEN_VX_MASK_FILE_WINDOW_BEGIN - Used to control the lower bound of the window around the valid time to determine if a GenVxMask input file should be used for processing. Overrides :term:`FILE_WINDOW_BEGIN`. See 'Use Windows to Find Valid Files' section for more information. + Used to control the lower bound of the window around the valid time to determine if a GenVxMask input file should + be used for processing. Applies to both the input file and the mask file(s). + Set :term:`GEN_VX_MASK_INPUT_FILE_WINDOW_BEGIN` or + :term:`GEN_VX_MASK_MASK_FILE_WINDOW_BEGIN` to set them separately. + Overrides :term:`FILE_WINDOW_BEGIN`. + See 'Use Windows to Find Valid Files' section for more information. | *Used by:* GenVxMask GEN_VX_MASK_FILE_WINDOW_END - Used to control the upper bound of the window around the valid time to determine if an GenVxMask input file should be used for processing. Overrides :term:`FILE_WINDOW_BEGIN`. See 'Use Windows to Find Valid Files' section for more information. + Used to control the upper bound of the window around the valid time to determine if an GenVxMask input file should + be used for processing. Applies to both the input file and the mask file(s). + Set :term:`GEN_VX_MASK_INPUT_FILE_WINDOW_END` or + :term:`GEN_VX_MASK_MASK_FILE_WINDOW_END` to set them separately. + Overrides :term:`FILE_WINDOW_END`. + See 'Use Windows to Find Valid Files' section for more information. + + | *Used by:* GenVxMask + + GEN_VX_MASK_INPUT_FILE_WINDOW_BEGIN + Used to control the lower bound of the window around the valid time to determine if a GenVxMask input file should + be used for processing. Applies to the input file only (not the mask file). + Set :term:`GEN_VX_MASK_FILE_WINDOW_BEGIN` to control both input and mask. + Overrides :term:`FILE_WINDOW_BEGIN`. + See 'Use Windows to Find Valid Files' section for more information. + + | *Used by:* GenVxMask + + GEN_VX_MASK_INPUT_FILE_WINDOW_END + Used to control the upper bound of the window around the valid time to determine if a GenVxMask input file should + be used for processing. Applies to the input file only (not the mask file). + Set :term:`GEN_VX_MASK_FILE_WINDOW_END` to control both input and mask. + Overrides :term:`FILE_WINDOW_END`. + See 'Use Windows to Find Valid Files' section for more information. + + | *Used by:* GenVxMask + + GEN_VX_MASK_MASK_FILE_WINDOW_BEGIN + Used to control the lower bound of the window around the valid time to determine if a GenVxMask mask file should + be used for processing. Applies to the mask file only (not the input file). + Set :term:`GEN_VX_MASK_FILE_WINDOW_BEGIN` to control both input and mask. + Overrides :term:`FILE_WINDOW_BEGIN`. + See 'Use Windows to Find Valid Files' section for more information. + + | *Used by:* GenVxMask + + GEN_VX_MASK_MASK_FILE_WINDOW_END + Used to control the upper bound of the window around the valid time to determine if a GenVxMask mask file should + be used for processing. Applies to the mask file only (not the input file). + Set :term:`GEN_VX_MASK_FILE_WINDOW_END` to control both input and mask. + Overrides :term:`FILE_WINDOW_END`. + See 'Use Windows to Find Valid Files' section for more information. | *Used by:* GenVxMask diff --git a/docs/Users_Guide/wrappers.rst b/docs/Users_Guide/wrappers.rst index dc3106fd0b..2218a9fe0a 100644 --- a/docs/Users_Guide/wrappers.rst +++ b/docs/Users_Guide/wrappers.rst @@ -1749,6 +1749,10 @@ Configuration | :term:`GEN_VX_MASK_CUSTOM_LOOP_LIST` | :term:`GEN_VX_MASK_FILE_WINDOW_BEGIN` | :term:`GEN_VX_MASK_FILE_WINDOW_END` +| :term:`GEN_VX_MASK_INPUT_FILE_WINDOW_BEGIN` +| :term:`GEN_VX_MASK_INPUT_FILE_WINDOW_END` +| :term:`GEN_VX_MASK_MASK_FILE_WINDOW_BEGIN` +| :term:`GEN_VX_MASK_MASK_FILE_WINDOW_END` | :term:`GEN_VX_MASK_SKIP_VALID_TIMES` | :term:`GEN_VX_MASK_INC_VALID_TIMES` | :term:`GEN_VX_MASK_SKIP_INIT_TIMES` 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 657a0e8642..efb33d9b22 100644 --- a/internal/tests/pytests/util/config_metplus/test_config_metplus.py +++ b/internal/tests/pytests/util/config_metplus/test_config_metplus.py @@ -68,64 +68,6 @@ def test_get_default_config_list(): assert actual_both == expected_both -@pytest.mark.parametrize( - 'regex,index,id,expected_result', [ - # 0: No ID - (r'^FCST_VAR(\d+)_NAME$', 1, None, - {'1': [None], - '2': [None], - '4': [None]}), - # 1: ID and index 2 - (r'(\w+)_VAR(\d+)_NAME', 2, 1, - {'1': ['FCST'], - '2': ['FCST'], - '4': ['FCST']}), - # 2: index 1, ID 2, multiple identifiers - (r'^FCST_VAR(\d+)_(\w+)$', 1, 2, - {'1': ['NAME', 'LEVELS'], - '2': ['NAME'], - '4': ['NAME']}), - # 3: command that StatAnalysis wrapper uses - (r'MODEL(\d+)$', 1, None, - {'1': [None], - '2': [None],}), - # 4: TCPairs conensus logic - (r'^TC_PAIRS_CONSENSUS(\d+)_(\w+)$', 1, 2, - {'1': ['NAME', 'MEMBERS', 'REQUIRED', 'MIN_REQ'], - '2': ['NAME', 'MEMBERS', 'REQUIRED', 'MIN_REQ']}), - ] -) -@pytest.mark.util -def test_find_indices_in_config_section(metplus_config, regex, index, - id, expected_result): - config = metplus_config - config.set('config', 'FCST_VAR1_NAME', 'name1') - config.set('config', 'FCST_VAR1_LEVELS', 'level1') - config.set('config', 'FCST_VAR2_NAME', 'name2') - config.set('config', 'FCST_VAR4_NAME', 'name4') - config.set('config', 'MODEL1', 'model1') - config.set('config', 'MODEL2', 'model2') - - config.set('config', 'TC_PAIRS_CONSENSUS1_NAME', 'name1') - config.set('config', 'TC_PAIRS_CONSENSUS1_MEMBERS', 'member1') - config.set('config', 'TC_PAIRS_CONSENSUS1_REQUIRED', 'True') - config.set('config', 'TC_PAIRS_CONSENSUS1_MIN_REQ', '1') - config.set('config', 'TC_PAIRS_CONSENSUS2_NAME', 'name2') - config.set('config', 'TC_PAIRS_CONSENSUS2_MEMBERS', 'member2') - config.set('config', 'TC_PAIRS_CONSENSUS2_REQUIRED', 'True') - config.set('config', 'TC_PAIRS_CONSENSUS2_MIN_REQ', '2') - - indices = config_metplus.find_indices_in_config_section(regex, config, - index_index=index, - id_index=id) - - pp = pprint.PrettyPrinter() - print(f'Indices:') - pp.pprint(indices) - - assert indices == expected_result - - @pytest.mark.parametrize( 'config_var_name, expected_indices, set_met_tool', [ ('FCST_GRID_STAT_VAR1_NAME', [1], True), @@ -409,14 +351,14 @@ def test_parse_var_list_both(metplus_config, data_type, list_created): var_list = config_metplus.parse_var_list(conf, time_info=None, data_type=data_type) print(f'var_list:{var_list}') for list_to_check in list_created.split(':'): - if (not var_list[0][f'{list_to_check}_name'] == "NAME1" or - not var_list[1][f'{list_to_check}_name'] == "NAME1" or - not var_list[2][f'{list_to_check}_name'] == "NAME2" or - not var_list[3][f'{list_to_check}_name'] == "NAME2" or - not var_list[0][f'{list_to_check}_level'] == "LEVELS11" or - not var_list[1][f'{list_to_check}_level'] == "LEVELS12" or - not var_list[2][f'{list_to_check}_level'] == "LEVELS21" or - not var_list[3][f'{list_to_check}_level'] == "LEVELS22"): + if (var_list[0][f'{list_to_check}_name'] != "NAME1" or + var_list[1][f'{list_to_check}_name'] != "NAME1" or + var_list[2][f'{list_to_check}_name'] != "NAME2" or + var_list[3][f'{list_to_check}_name'] != "NAME2" or + var_list[0][f'{list_to_check}_level'] != "LEVELS11" or + var_list[1][f'{list_to_check}_level'] != "LEVELS12" or + var_list[2][f'{list_to_check}_level'] != "LEVELS21" or + var_list[3][f'{list_to_check}_level'] != "LEVELS22"): assert False @@ -517,8 +459,6 @@ def test_parse_var_list_fcst_and_obs_and_both(metplus_config, data_type, list_le if expect[f'{dt_lower}_level'] != reality[f'{dt_lower}_level']: assert False - assert True - # option defined in obs only @pytest.mark.parametrize( @@ -646,9 +586,9 @@ def test_parse_var_list_ensemble(metplus_config): met_tool='ensemble_stat') pp = pprint.PrettyPrinter() - print(f'ENSEMBLE_VAR_LIST:') + print('ENSEMBLE_VAR_LIST:') pp.pprint(ensemble_var_list) - print(f'VAR_LIST:') + print('VAR_LIST:') pp.pprint(var_list) assert(len(ensemble_var_list) == len(expected_ens_list)) @@ -715,9 +655,9 @@ def test_parse_var_list_series_by(metplus_config): met_tool='series_analysis') pp = pprint.PrettyPrinter() - print(f'ExtractTiles var list:') + print('ExtractTiles var list:') pp.pprint(actual_et_list) - print(f'SeriesAnalysis var list:') + print('SeriesAnalysis var list:') pp.pprint(actual_sa_list) assert len(actual_et_list) == len(expected_et_list) @@ -898,7 +838,6 @@ def test_getraw_instance_with_unset_var(metplus_config): """! Replicates bug where CURRENT_FCST_NAME is substituted with an empty string when copied from an instance section """ - pytest.skip() instance = 'my_section' config = metplus_config config.set('config', 'MODEL', 'FCST') 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 e448eb2ba0..be7d6c63b4 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 @@ -5,7 +5,8 @@ import pprint from datetime import datetime -from metplus.util.string_manip import * +from metplus.util.string_manip import get_log_path, get_logfile_info, template_to_regex, subset_list, expand_int_string_to_list, round_0p5, validate_thresholds, get_threshold_via_regex, camel_to_underscore, remove_quotes, getlist, getlistint, list_to_str, comparison_to_letter_format, format_thresh, format_level, find_indices_in_config_section + @pytest.mark.parametrize( @@ -373,7 +374,7 @@ def test_getlist_begin_end_incr(list_string, output_list): @pytest.mark.parametrize( - 'input, add_quotes, expected_output', [ + 'input_list, add_quotes, expected_output', [ (['a', 'b', 'c'], None, '"a", "b", "c"'), (['0', '1', '2'], None, '"0", "1", "2"'), (['a', 'b', 'c'], True, '"a", "b", "c"'), @@ -385,11 +386,11 @@ def test_getlist_begin_end_incr(list_string, output_list): ] ) @pytest.mark.util -def test_list_to_str(input, add_quotes, expected_output): +def test_list_to_str(input_list, add_quotes, expected_output): if add_quotes is None: - assert list_to_str(input) == expected_output + assert list_to_str(input_list) == expected_output else: - assert list_to_str(input, add_quotes=add_quotes) == expected_output + assert list_to_str(input_list, add_quotes=add_quotes) == expected_output @pytest.mark.parametrize( @@ -440,7 +441,7 @@ def test_format_level(level, expected_result): @pytest.mark.parametrize( - 'regex,index,id,expected_result', [ + 'regex,index,id_index,expected_result', [ # 0: No ID (r'^FCST_VAR(\d+)_NAME$', 1, None, {'1': [None], @@ -468,7 +469,7 @@ def test_format_level(level, expected_result): ) @pytest.mark.util def test_find_indices_in_config_section(metplus_config, regex, index, - id, expected_result): + id_index, expected_result): config = metplus_config config.set('config', 'FCST_VAR1_NAME', 'name1') config.set('config', 'FCST_VAR1_LEVELS', 'level1') @@ -487,10 +488,10 @@ def test_find_indices_in_config_section(metplus_config, regex, index, config.set('config', 'TC_PAIRS_CONSENSUS2_MIN_REQ', '2') indices = find_indices_in_config_section(regex, config, index_index=index, - id_index=id) + id_index=id_index) pp = pprint.PrettyPrinter() - print(f'Indices:') + print('Indices:') pp.pprint(indices) assert indices == expected_result diff --git a/internal/tests/pytests/wrappers/gen_vx_mask/test_gen_vx_mask.py b/internal/tests/pytests/wrappers/gen_vx_mask/test_gen_vx_mask.py index 3c745d4c64..5ecc579196 100644 --- a/internal/tests/pytests/wrappers/gen_vx_mask/test_gen_vx_mask.py +++ b/internal/tests/pytests/wrappers/gen_vx_mask/test_gen_vx_mask.py @@ -20,6 +20,34 @@ def gen_vx_mask_wrapper(metplus_config): config.set('config', 'DO_NOT_RUN_EXE', True) return GenVxMaskWrapper(config) +@pytest.mark.parametrize( + 'input_val, expected', + [ + # no input returns a list with an empty string + ('', [''] ), + # two sets of options as demonstrated in GenVxMask_multiple.conf + ("-type lat -thresh 'ge30&&le50', -type lon -thresh 'le-70&&ge-130' -intersection -name lat_lon_mask", + ["-type lat -thresh 'ge30&&le50'", + "-type lon -thresh 'le-70&&ge-130' -intersection -name lat_lon_mask"] ), + # first item in list must be a flag starting with -, e.g. -type + ("type lat -thresh 'ge30&&le50'", + ["error"] ), + # complex single set of options that includes multiple values for -type/-thresh/etc as recently added in dtcenter/MET#3008 + ('-type data,data,lat,lon -mask_field \'name="LAND"; level="L0";\' -mask_field \'name="TMP"; level="L0";\' -thresh eq1,lt273,gt0,lt0 -intersection -v 5', + ['-type data,data,lat,lon -mask_field \'name="LAND"; level="L0";\' -mask_field \'name="TMP"; level="L0";\' -thresh eq1,lt273,gt0,lt0 -intersection -v 5'] ), + # two sets of options with one of them containing multiple values for -type/-thresh/etc + ('-type data -mask_field \'name="LAND"; level="L0";\' -thresh eq1, -type data,lat,lon -mask_field \'name="TMP"; level="L0";\' -thresh lt273,gt0,lt0 -intersection -v 5', + ['-type data -mask_field \'name="LAND"; level="L0";\' -thresh eq1', + '-type data,lat,lon -mask_field \'name="TMP"; level="L0";\' -thresh lt273,gt0,lt0 -intersection -v 5']), + ] +) +@pytest.mark.wrapper +def test_handle_command_options(metplus_config, input_val, expected): + config = metplus_config + wrapper = GenVxMaskWrapper(config) + actual = wrapper.parse_command_options_list(input_val) + assert actual == expected + @pytest.mark.parametrize( 'missing, run, thresh, errors, allow_missing', [ @@ -78,9 +106,8 @@ def test_run_gen_vx_mask_once(metplus_config): 'GenVxMask_test') wrap.c_dict['OUTPUT_TEMPLATE'] = '{valid?fmt=%Y%m%d%H}_ZENITH_LAT_MASK.nc' wrap.c_dict['COMMAND_OPTIONS'] = ["-type lat -thresh 'ge30&&le50'"] -# wrap.c_dict['MASK_INPUT_TEMPLATES'] = ['LAT', 'LON'] -# wrap.c_dict['COMMAND_OPTIONS'] = ["-type lat -thresh 'ge30&&le50'", "-type lon -thresh 'le-70&&ge-130' -intersection"] + wrap.c_dict['ALL_FILES'] = wrap.get_all_files_for_each(time_info) wrap.run_at_time_once(time_info) expected_cmd = f"{wrap.app_path} \"2018020100_ZENITH\" LAT {wrap.config.getdir('OUTPUT_BASE')}/GenVxMask_test/2018020100_ZENITH_LAT_MASK.nc -type lat -thresh 'ge30&&le50' -v 2" @@ -96,16 +123,22 @@ def test_run_gen_vx_mask_twice(metplus_config): input_dict = {'valid': datetime.datetime.strptime("201802010000",'%Y%m%d%H%M'), 'lead': 0} time_info = time_util.ti_calculate(input_dict) + cmd_args = [ + "-type lat -thresh 'ge30&&le50'", + "-type lon -thresh 'le-70&&ge-130' -intersection -name lat_lon_mask" + ] - wrap = gen_vx_mask_wrapper(metplus_config) - wrap.c_dict['INPUT_TEMPLATE'] = '{valid?fmt=%Y%m%d%H}_ZENITH' - wrap.c_dict['MASK_INPUT_TEMPLATES'] = ['LAT', 'LON'] - wrap.c_dict['OUTPUT_DIR'] = os.path.join(wrap.config.getdir('OUTPUT_BASE'), - 'GenVxMask_test') - wrap.c_dict['OUTPUT_TEMPLATE'] = '{valid?fmt=%Y%m%d%H}_ZENITH_LAT_LON_MASK.nc' - cmd_args = ["-type lat -thresh 'ge30&&le50'", "-type lon -thresh 'le-70&&ge-130' -intersection -name lat_lon_mask"] - wrap.c_dict['COMMAND_OPTIONS'] = cmd_args + config = metplus_config + + output_dir = os.path.join(config.getdir('OUTPUT_BASE'), 'GenVxMask_test') + config.set('config', 'GEN_VX_MASK_INPUT_TEMPLATE', '{valid?fmt=%Y%m%d%H}_ZENITH') + config.set('config', 'GEN_VX_MASK_INPUT_MASK_TEMPLATE', 'LAT, LON') + config.set('config', 'GEN_VX_MASK_OUTPUT_DIR', output_dir) + config.set('config', 'GEN_VX_MASK_OUTPUT_TEMPLATE', '{valid?fmt=%Y%m%d%H}_ZENITH_LAT_LON_MASK.nc') + config.set('config', 'GEN_VX_MASK_OPTIONS', ','.join(cmd_args)) + wrap = gen_vx_mask_wrapper(config) + wrap.c_dict['ALL_FILES'] = wrap.get_all_files_for_each(time_info) wrap.run_at_time_once(time_info) expected_cmds = [f"{wrap.app_path} \"2018020100_ZENITH\" LAT {wrap.config.getdir('OUTPUT_BASE')}/stage/gen_vx_mask/temp_0.nc {cmd_args[0]} -v 2", diff --git a/metplus/util/string_manip.py b/metplus/util/string_manip.py index 19342610cc..553c349c63 100644 --- a/metplus/util/string_manip.py +++ b/metplus/util/string_manip.py @@ -7,7 +7,6 @@ import sys import os import re -from csv import reader import random import string import logging @@ -45,7 +44,7 @@ def remove_quotes(input_string): def getlist(list_str, expand_begin_end_incr=True): - """! Returns a list of string elements from a comma + """!Returns a list of string elements from a comma separated string of values. This function MUST also return an empty list [] if s is '' empty. This function is meant to handle these possible or similar inputs: @@ -57,6 +56,8 @@ def getlist(list_str, expand_begin_end_incr=True): a conf file returns '' an empty string. @param list_str the string being converted to a list. + @param expand_begin_end_incr If False, do not expand begin_end_incr + notation into a list of items (defaults to True). @returns list of strings formatted properly and expanded as needed """ if not list_str: @@ -81,7 +82,7 @@ def getlist(list_str, expand_begin_end_incr=True): # use regex split to split list string by commas that are not # found within []s or ()s - item_list = re.split(r',\s*(?![^\[\]]*\]|[^()]*\))', list_str) + item_list = re.split(r',\s*(?![^\[\]]*]|[^()]*\))', list_str) # regex split will still split by commas that are found between # quotation marks, so call function to put them back together properly @@ -177,7 +178,7 @@ def _begin_end_incr_evaluate(item): def _fix_list(item_list): - """! The logic that calls this function may have incorrectly split up + """!The logic that calls this function may have incorrectly split up a string that contains commas within quotation marks. This function looks through the list and finds items that appear to have been split up incorrectly and puts them back together properly. @@ -196,11 +197,12 @@ def _fix_list(item_list): # otherwise add it to the list buffer else: list_buffer.append(item) - else: - list_buffer.append(item) - if quote_count == 1: - fixed_list.append(','.join(list_buffer)) - list_buffer.clear() + continue + + list_buffer.append(item) + if quote_count == 1: + fixed_list.append(','.join(list_buffer)) + list_buffer.clear() # if there are still items in the buffer, add them to end of list if list_buffer: @@ -210,9 +212,8 @@ def _fix_list(item_list): out_list = [] for item in fixed_list: if item[0] == '"' and item[-1] == '"': - out_list.append(item.strip('"')) - else: - out_list.append(item) + item = item.strip('"') + out_list.append(item) return out_list @@ -334,18 +335,20 @@ def get_threshold_via_regex(thresh_string): break match = re.match(r'^('+comp+r')(.*\d.*)$', thresh) - if match: - comparison = match.group(1) - number = match.group(2) - # try to convert to float if it can, but allow string - try: - number = float(number) - except ValueError: - pass - - comparison_number_list.append((comparison, number)) - found_match = True - break + if not match: + continue + + comparison = match.group(1) + number = match.group(2) + # try to convert to float if it can, but allow string + try: + number = float(number) + except ValueError: + pass + + comparison_number_list.append((comparison, number)) + found_match = True + break # if no match was found for the item, return None if not found_match: diff --git a/metplus/wrappers/command_builder.py b/metplus/wrappers/command_builder.py index d47efa2617..9292bbd73d 100755 --- a/metplus/wrappers/command_builder.py +++ b/metplus/wrappers/command_builder.py @@ -332,6 +332,7 @@ def handle_file_window_variables(self, c_dict, data_types=None): for edge in edges: input_list = [ f'{data_type}_{app}_FILE_WINDOW_{edge}', + f'{app}_{data_type}_FILE_WINDOW_{edge}', f'{app}_FILE_WINDOW_{edge}', f'{data_type}_FILE_WINDOW_{edge}', f'FILE_WINDOW_{edge}', diff --git a/metplus/wrappers/gen_vx_mask_wrapper.py b/metplus/wrappers/gen_vx_mask_wrapper.py index 59ef9e2f4c..2e07408058 100755 --- a/metplus/wrappers/gen_vx_mask_wrapper.py +++ b/metplus/wrappers/gen_vx_mask_wrapper.py @@ -41,33 +41,29 @@ def create_c_dict(self): c_dict['ALLOW_MULTIPLE_FILES'] = False # input and output files - c_dict['INPUT_DIR'] = self.config.getdir('GEN_VX_MASK_INPUT_DIR', '') - c_dict['INPUT_TEMPLATE'] = self.config.getraw('config', - 'GEN_VX_MASK_INPUT_TEMPLATE') + self.get_input_templates(c_dict, { + 'INPUT': {'prefix': 'GEN_VX_MASK', 'required': True}, + }) c_dict['OUTPUT_DIR'] = self.config.getdir('GEN_VX_MASK_OUTPUT_DIR', '') - c_dict['OUTPUT_TEMPLATE'] = self.config.getraw('config', - 'GEN_VX_MASK_OUTPUT_TEMPLATE') + c_dict['OUTPUT_TEMPLATE'] = self.config.getraw('config', 'GEN_VX_MASK_OUTPUT_TEMPLATE') - c_dict['MASK_INPUT_DIR'] = self.config.getdir('GEN_VX_MASK_INPUT_MASK_DIR', - '') + # Cannot use get_input_templates to set mask dir/template because + # config variable names do not end with INPUT_DIR and INPUT_TEMPLATE. + # Also, mask templates are looped over for separate calls to gen_vx_mask + # instead of passing all files from template to a single command. + c_dict['MASK_INPUT_DIR'] = self.config.getdir('GEN_VX_MASK_INPUT_MASK_DIR', '') c_dict['MASK_INPUT_TEMPLATES'] = getlist( self.config.getraw('config', 'GEN_VX_MASK_INPUT_MASK_TEMPLATE') ) if not c_dict['MASK_INPUT_TEMPLATES']: self.log_error("Must set GEN_VX_MASK_INPUT_MASK_TEMPLATE to run GenVxMask wrapper") - self.isOK = False - # optional arguments - c_dict['COMMAND_OPTIONS'] = getlist( + c_dict['COMMAND_OPTIONS'] = self.parse_command_options_list( self.config.getraw('config', 'GEN_VX_MASK_OPTIONS') ) - # if no options were specified, set to a list with an empty string - if not c_dict['COMMAND_OPTIONS']: - c_dict['COMMAND_OPTIONS'] = [''] - # error if -type is not set (previously optional) if not any([item for item in c_dict['COMMAND_OPTIONS'] if '-type' in item]): self.log_error("Must specify -type in GEN_VX_MASK_OPTIONS") @@ -78,75 +74,76 @@ def create_c_dict(self): self.log_error("Number of items in GEN_VX_MASK_INPUT_MASK_TEMPLATE must " "be equal to the number of items in GEN_VX_MASK_OPTIONS") - self.isOK = False - # handle window variables [GEN_VX_MASK_]FILE_WINDOW_[BEGIN/END] - c_dict['FILE_WINDOW_BEGIN'] = \ - self.config.getseconds('config', 'GEN_VX_MASK_FILE_WINDOW_BEGIN', - self.config.getseconds('config', - 'FILE_WINDOW_BEGIN', 0)) - c_dict['FILE_WINDOW_END'] = \ - self.config.getseconds('config', 'GEN_VX_MASK_FILE_WINDOW_END', - self.config.getseconds('config', - 'FILE_WINDOW_END', 0)) - - # use the same file windows for input and mask files - c_dict['MASK_FILE_WINDOW_BEGIN'] = c_dict['FILE_WINDOW_BEGIN'] - c_dict['MASK_FILE_WINDOW_END'] = c_dict['FILE_WINDOW_END'] - # skip RuntimeFreq input file logic - remove once integrated - c_dict['FIND_FILES'] = False - return c_dict - - def get_command(self): - cmd = self.app_path + # or separately for input file (INPUT) or mask file (MASK) + self.handle_file_window_variables(c_dict, data_types=['INPUT', 'MASK']) - # don't run if no input or output files were found - if not self.infiles: - self.log_error("No input files were found") - return - - if not self.outfile: - self.log_error("No output file specified") - return - - # add input files - for infile in self.infiles: - cmd += ' ' + infile + return c_dict - # add output path - out_path = self.get_output_path() - cmd += ' ' + out_path + def parse_command_options_list(self, options_text): + """!Split string of command line options into a list. First use getlist + function to preserve commas within quotation marks, e.g. NetCDF field + info. Option values can now support a comma-separated list of values + without quotation marks, so put back together list items that were + incorrectly split apart, e.g. "-type lat,lon" + + @param options_text: String of command line options separated by comma + @returns list containing groups of command line options, or a list with + an empty string if no options were provided, or a list with the string + 'error' if invalid options were provided, e.g. does not start with a + dash. + """ + command_options = getlist(options_text) - # add arguments - cmd += ' ' + self.args + # if no options were specified, set to a list with an empty string + if not command_options: + return [''] + + # first value of command options must start with -, e.g. -type + if command_options[0][0] != '-': + self.log_error('Invalid GEN_VX_MASK_OPTIONS: Must start with a ' + 'flag, e.g. -type') + return ['error'] + + # combine list items that may have been incorrectly split + # since -type/-thresh/etc options now support a comma-separated list + fixed_options = [] + for option in command_options: + if option.startswith('-'): + fixed_options.append(option) + else: + fixed_options[-1] += f',{option}' + + return fixed_options - # add verbosity - cmd += ' -v ' + self.c_dict['VERBOSITY'] - return cmd + def get_command(self): + return ( + f"{self.app_path} {' '.join(self.infiles)} {self.get_output_path()}" + f"{' ' + ' '.join(self.args) if self.args else ''}" + f" -v {self.c_dict['VERBOSITY']}" + ) def run_at_time_once(self, time_info): - """!Loop over list of mask templates and call GenVxMask for each, adding the - corresponding command line arguments for each call - Args: - @param time_info time dictionary for current runtime - @returns None + """!Loop over list of mask templates and call GenVxMask for each, + adding the corresponding command line arguments for each call + + @param time_info time dictionary for current runtime + @returns None """ # set environment variables # there is no config file, so using CommandBuilder implementation self.set_environment_variables(time_info) - # loop over mask templates and command line args, - self.run_count += 1 + # loop over mask templates and command line args temp_file = '' for index, (mask_template, cmd_args) in enumerate(zip(self.c_dict['MASK_INPUT_TEMPLATES'], self.c_dict['COMMAND_OPTIONS'])): # set mask input template and command line arguments self.c_dict['MASK_INPUT_TEMPLATE'] = mask_template - self.args = do_string_sub(cmd_args, **time_info) + self.args = [do_string_sub(cmd_args, **time_info)] if not self.find_input_files(time_info, temp_file): - self.missing_input_count += 1 return # break out of loop if this is the last iteration to @@ -183,18 +180,17 @@ def find_input_files(self, time_info, temp_file): # clear out input file list self.infiles.clear() + # if temp file is set, use that as input + input_path = temp_file + # if temp file is not set, this is the first iteration, so read input file if not temp_file: - input_path = self.find_data(time_info) + input_path = self.c_dict['ALL_FILES'][0].get('INPUT', [''])[0] # return if file was not found if not input_path: return None - # if temp file is set, use that as input - else: - input_path = temp_file - # find mask file, using MASK_INPUT_TEMPLATE mask_file = self.find_data(time_info, data_type='MASK') if not mask_file: diff --git a/metplus/wrappers/runtime_freq_wrapper.py b/metplus/wrappers/runtime_freq_wrapper.py index 57a3f76d26..89f0ec7f56 100755 --- a/metplus/wrappers/runtime_freq_wrapper.py +++ b/metplus/wrappers/runtime_freq_wrapper.py @@ -116,6 +116,8 @@ def get_input_templates(self, c_dict, input_info=None): return for label, info in input_info.items(): + if label: + label = f'{label}_' prefix = info.get('prefix') required = info.get('required', True) @@ -124,18 +126,18 @@ def get_input_templates(self, c_dict, input_info=None): c_dict['EXPLICIT_FILE_LIST'] = True else: input_dir = self.config.getdir(f'{prefix}_INPUT_DIR', '') - c_dict[f'{label}_INPUT_DIR'] = input_dir + c_dict[f'{label}INPUT_DIR'] = input_dir templates = getlist( self.config.getraw('config', f'{prefix}_INPUT_TEMPLATE') ) template = ','.join(templates) - c_dict[f'{label}_INPUT_TEMPLATE'] = template - if not c_dict[f'{label}_INPUT_TEMPLATE']: + c_dict[f'{label}INPUT_TEMPLATE'] = template + if not c_dict[f'{label}INPUT_TEMPLATE']: if required: self.log_error(f'{prefix}_INPUT_TEMPLATE required to run') continue - template_dict[label] = (template, True) + template_dict[label.rstrip('_')] = (template, True, False) c_dict['TEMPLATE_DICT'] = template_dict @@ -169,7 +171,7 @@ def get_input_templates_multiple(self, c_dict): else: label = input_template_labels[idx] - template_dict[label] = (template, False) + template_dict[label] = (template, False, True) c_dict['TEMPLATE_DICT'] = template_dict @@ -640,10 +642,10 @@ def get_input_files(self, time_info, fill_missing=False): return None, time_info offset_time_info = time_info - for label, (template, required) in self.c_dict['TEMPLATE_DICT'].items(): + for label, (template, required, uses_custom_labels) in self.c_dict['TEMPLATE_DICT'].items(): data_type = '' template_key = 'INPUT_TEMPLATE' - if label in ('FCST', 'OBS'): + if not uses_custom_labels: data_type = label template_key = f'{label}_{template_key}'