Skip to content

Commit

Permalink
per #2586, added function with tests to properly parse list of comman…
Browse files Browse the repository at this point in the history
…d line arguments that can now contain comma-separated lists that should not be split up into separate items
  • Loading branch information
georgemccabe committed Dec 13, 2024
1 parent 1105648 commit 191d858
Show file tree
Hide file tree
Showing 3 changed files with 66 additions and 9 deletions.
28 changes: 28 additions & 0 deletions internal/tests/pytests/wrappers/gen_vx_mask/test_gen_vx_mask.py
Original file line number Diff line number Diff line change
Expand Up @@ -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', [
Expand Down
2 changes: 1 addition & 1 deletion metplus/util/string_manip.py
Original file line number Diff line number Diff line change
Expand Up @@ -178,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.
Expand Down
45 changes: 37 additions & 8 deletions metplus/wrappers/gen_vx_mask_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -57,17 +57,12 @@ def create_c_dict(self):

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")
Expand All @@ -78,8 +73,6 @@ 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',
Expand All @@ -97,6 +90,42 @@ def create_c_dict(self):
c_dict['FIND_FILES'] = False
return c_dict

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)

# 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

def get_command(self):
cmd = self.app_path

Expand Down

0 comments on commit 191d858

Please sign in to comment.