Skip to content

Commit

Permalink
Bugfix #2070 var list numeric order (#2072)
Browse files Browse the repository at this point in the history
  • Loading branch information
@georgemccabe
georgemccabe authored Mar 10, 2023
1 parent b0aec2f commit c19aebc
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 83 deletions.
155 changes: 88 additions & 67 deletions internal/tests/pytests/util/config_metplus/test_config_metplus.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,18 @@ def test_find_indices_in_config_section(metplus_config, regex, index,

@pytest.mark.parametrize(
'config_var_name, expected_indices, set_met_tool', [
('FCST_GRID_STAT_VAR1_NAME', ['1'], True),
('FCST_GRID_STAT_VAR2_INPUT_FIELD_NAME', ['2'], True),
('FCST_GRID_STAT_VAR3_FIELD_NAME', ['3'], True),
('BOTH_GRID_STAT_VAR4_NAME', ['4'], True),
('BOTH_GRID_STAT_VAR5_INPUT_FIELD_NAME', ['5'], True),
('BOTH_GRID_STAT_VAR6_FIELD_NAME', ['6'], True),
('FCST_VAR7_NAME', ['7'], False),
('FCST_VAR8_INPUT_FIELD_NAME', ['8'], False),
('FCST_VAR9_FIELD_NAME', ['9'], False),
('BOTH_VAR10_NAME', ['10'], False),
('BOTH_VAR11_INPUT_FIELD_NAME', ['11'], False),
('BOTH_VAR12_FIELD_NAME', ['12'], False),
('FCST_GRID_STAT_VAR1_NAME', [1], True),
('FCST_GRID_STAT_VAR2_INPUT_FIELD_NAME', [2], True),
('FCST_GRID_STAT_VAR3_FIELD_NAME', [3], True),
('BOTH_GRID_STAT_VAR4_NAME', [4], True),
('BOTH_GRID_STAT_VAR5_INPUT_FIELD_NAME', [5], True),
('BOTH_GRID_STAT_VAR6_FIELD_NAME', [6], True),
('FCST_VAR7_NAME', [7], False),
('FCST_VAR8_INPUT_FIELD_NAME', [8], False),
('FCST_VAR9_FIELD_NAME', [9], False),
('BOTH_VAR10_NAME', [10], False),
('BOTH_VAR11_INPUT_FIELD_NAME', [11], False),
('BOTH_VAR12_FIELD_NAME', [12], False),
]
)
@pytest.mark.util
Expand All @@ -126,12 +126,14 @@ def test_find_var_indices_fcst(metplus_config,
data_types = ['FCST']
config.set('config', config_var_name, "NAME1")
met_tool = 'grid_stat' if set_met_tool else None
var_name_indices = config_metplus._find_var_name_indices(config,
data_types=data_types,
met_tool=met_tool)
actual_indices = (
config_metplus._find_var_name_indices(config,
data_types=data_types,
met_tool=met_tool)
)

assert len(var_name_indices) == len(expected_indices)
for actual_index in var_name_indices:
assert len(actual_indices) == len(expected_indices)
for actual_index in actual_indices:
assert actual_index in expected_indices


Expand Down Expand Up @@ -347,13 +349,13 @@ def test_parse_var_list_obs(metplus_config, data_type, list_created):
# list will be created if requesting just OBS, but it should not be created if
# nothing was requested because FCST values are missing
if list_created:
assert(var_list[0]['obs_name'] == "NAME1" and \
var_list[1]['obs_name'] == "NAME1" and \
var_list[2]['obs_name'] == "NAME2" and \
var_list[3]['obs_name'] == "NAME2" and \
var_list[0]['obs_level'] == "LEVELS11" and \
var_list[1]['obs_level'] == "LEVELS12" and \
var_list[2]['obs_level'] == "LEVELS21" and \
assert(var_list[0]['obs_name'] == "NAME1" and
var_list[1]['obs_name'] == "NAME1" and
var_list[2]['obs_name'] == "NAME2" and
var_list[3]['obs_name'] == "NAME2" and
var_list[0]['obs_level'] == "LEVELS11" and
var_list[1]['obs_level'] == "LEVELS12" and
var_list[2]['obs_level'] == "LEVELS21" and
var_list[3]['obs_level'] == "LEVELS22")
else:
assert not var_list
Expand Down Expand Up @@ -382,15 +384,15 @@ 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":
assert False
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"):
assert False


# field info defined in both FCST_* and OBS_* variables
Expand All @@ -412,21 +414,21 @@ def test_parse_var_list_fcst_and_obs(metplus_config):

var_list = config_metplus.parse_var_list(conf)

assert(var_list[0]['fcst_name'] == "FNAME1" and \
var_list[0]['obs_name'] == "ONAME1" and \
var_list[1]['fcst_name'] == "FNAME1" and \
var_list[1]['obs_name'] == "ONAME1" and \
var_list[2]['fcst_name'] == "FNAME2" and \
var_list[2]['obs_name'] == "ONAME2" and \
var_list[3]['fcst_name'] == "FNAME2" and \
var_list[3]['obs_name'] == "ONAME2" and \
var_list[0]['fcst_level'] == "FLEVELS11" and \
var_list[0]['obs_level'] == "OLEVELS11" and \
var_list[1]['fcst_level'] == "FLEVELS12" and \
var_list[1]['obs_level'] == "OLEVELS12" and \
var_list[2]['fcst_level'] == "FLEVELS21" and \
var_list[2]['obs_level'] == "OLEVELS21" and \
var_list[3]['fcst_level'] == "FLEVELS22" and \
assert(var_list[0]['fcst_name'] == "FNAME1" and
var_list[0]['obs_name'] == "ONAME1" and
var_list[1]['fcst_name'] == "FNAME1" and
var_list[1]['obs_name'] == "ONAME1" and
var_list[2]['fcst_name'] == "FNAME2" and
var_list[2]['obs_name'] == "ONAME2" and
var_list[3]['fcst_name'] == "FNAME2" and
var_list[3]['obs_name'] == "ONAME2" and
var_list[0]['fcst_level'] == "FLEVELS11" and
var_list[0]['obs_level'] == "OLEVELS11" and
var_list[1]['fcst_level'] == "FLEVELS12" and
var_list[1]['obs_level'] == "OLEVELS12" and
var_list[2]['fcst_level'] == "FLEVELS21" and
var_list[2]['obs_level'] == "OLEVELS21" and
var_list[3]['fcst_level'] == "FLEVELS22" and
var_list[3]['obs_level'] == "OLEVELS22")


Expand Down Expand Up @@ -520,22 +522,25 @@ def test_parse_var_list_fcst_only_options(metplus_config, data_type, list_len):

@pytest.mark.parametrize(
'met_tool, indices', [
(None, {'1': ['FCST']}),
('GRID_STAT', {'2': ['FCST']}),
('ENSEMBLE_STAT', {}),
(None, [1]),
('GRID_STAT', [2]),
('ENSEMBLE_STAT', []),
]
)
@pytest.mark.util
def test_find_var_indices_wrapper_specific(metplus_config, met_tool, indices):
conf = metplus_config
config = metplus_config
data_type = 'FCST'
conf.set('config', f'{data_type}_VAR1_NAME', "NAME1")
conf.set('config', f'{data_type}_GRID_STAT_VAR2_NAME', "GSNAME2")
config.set('config', f'{data_type}_VAR1_NAME', "NAME1")
config.set('config', f'{data_type}_GRID_STAT_VAR2_NAME', "GSNAME2")

var_name_indices = config_metplus._find_var_name_indices(conf,data_types=[data_type],
met_tool=met_tool)
actual_indices = (
config_metplus._find_var_name_indices(config,
data_types=[data_type],
met_tool=met_tool)
)

assert var_name_indices == indices
assert actual_indices == indices


# ensure that the field configuration used for
Expand Down Expand Up @@ -572,28 +577,28 @@ def test_parse_var_list_ensemble(metplus_config):
'ens_phist_bin_size = 0.05;'))
time_info = {}

expected_ens_list = [{'index': '1',
expected_ens_list = [{'index': 1,
'ens_name': 'APCP',
'ens_level': 'A24',
'ens_thresh': ['>0.0', '>=10.0']},
{'index': '2',
{'index': 2,
'ens_name': 'REFC',
'ens_level': 'L0',
'ens_thresh': ['>35.0']},
{'index': '3',
{'index': 3,
'ens_name': 'UGRD',
'ens_level': 'Z10',
'ens_thresh': ['>=5.0']},
{'index': '4',
{'index': 4,
'ens_name': 'VGRD',
'ens_level': 'Z10',
'ens_thresh': ['>=5.0']},
{'index': '5',
{'index': 5,
'ens_name': 'WIND',
'ens_level': 'Z10',
'ens_thresh': ['>=5.0']},
]
expected_var_list = [{'index': '1',
expected_var_list = [{'index': 1,
'fcst_name': 'APCP',
'fcst_level': 'A24',
'fcst_thresh': ['>0.01', '>=10.0'],
Expand Down Expand Up @@ -646,15 +651,15 @@ def test_parse_var_list_series_by(metplus_config):
config.set('config', 'BOTH_SERIES_ANALYSIS_VAR2_LEVELS', 'P700')
time_info = {}

expected_et_list = [{'index': '1',
expected_et_list = [{'index': 1,
'fcst_name': 'RH',
'fcst_level': 'P850',
'fcst_output_name': 'RH_850mb',
'obs_name': 'RH',
'obs_level': 'P850',
'obs_output_name': 'RH_850mb',
},
{'index': '1',
{'index': 1,
'fcst_name': 'RH',
'fcst_level': 'P700',
'fcst_output_name': 'RH_700mb',
Expand All @@ -663,13 +668,13 @@ def test_parse_var_list_series_by(metplus_config):
'obs_output_name': 'RH_700mb',
},
]
expected_sa_list = [{'index': '1',
expected_sa_list = [{'index': 1,
'fcst_name': 'RH_850mb',
'fcst_level': 'P850',
'obs_name': 'RH_850mb',
'obs_level': 'P850',
},
{'index': '2',
{'index': 2,
'fcst_name': 'RH_700mb',
'fcst_level': 'P700',
'obs_name': 'RH_700mb',
Expand Down Expand Up @@ -913,3 +918,19 @@ def test_format_var_items_options_semicolon(config_value,
var_items = config_metplus._format_var_items(field_configs, time_info)
result = var_items.get('extra')
assert result == expected_result


@pytest.mark.util
def test_parse_var_list_double_digit(metplus_config):
"""!This test ensures that parse_var_list returns field info in
numeric order (1,2,...,9,10,11) instead of alphabetical (1,10,11,2,3,etc)
"""
config = metplus_config
for n in range(1, 12, 1):
config.set('config', f'FCST_VAR{n}_NAME', f'fcst_name{n}')
config.set('config', f'OBS_VAR{n}_NAME', f'obs_name{n}')

var_list = config_metplus.parse_var_list(config)
for n, var_item in enumerate(var_list, start=1):
assert var_item['fcst_name'] == f'fcst_name{n}'
assert var_item['obs_name'] == f'obs_name{n}'
8 changes: 4 additions & 4 deletions internal/tests/pytests/util/config_util/test_config_util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -115,13 +115,13 @@ def test_get_process_list_instances(metplus_config, input_list, expected_list):
({'init': datetime(2019, 2, 1, 6),
'lead': 7200, },
[
{'index': '1',
{'index': 1,
'fcst_name': 'FNAME_2019',
'fcst_level': 'Z06',
'obs_name': 'ONAME_2019',
'obs_level': 'L06',
},
{'index': '1',
{'index': 1,
'fcst_name': 'FNAME_2019',
'fcst_level': 'Z08',
'obs_name': 'ONAME_2019',
Expand All @@ -131,13 +131,13 @@ def test_get_process_list_instances(metplus_config, input_list, expected_list):
({'init': datetime(2021, 4, 13, 9),
'lead': 10800, },
[
{'index': '1',
{'index': 1,
'fcst_name': 'FNAME_2021',
'fcst_level': 'Z09',
'obs_name': 'ONAME_2021',
'obs_level': 'L09',
},
{'index': '1',
{'index': 1,
'fcst_name': 'FNAME_2021',
'fcst_level': 'Z12',
'obs_name': 'ONAME_2021',
Expand Down
32 changes: 24 additions & 8 deletions metplus/util/config_metplus.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -942,9 +942,9 @@ def parse_var_list(config, time_info=None, data_type=None, met_tool=None,
# get indices of VAR<n> items for data type and/or met tool
indices = []
if met_tool:
indices = _find_var_name_indices(config, data_types, met_tool).keys()
indices = _find_var_name_indices(config, data_types, met_tool)
if not indices:
indices = _find_var_name_indices(config, data_types).keys()
indices = _find_var_name_indices(config, data_types)

# get config name prefixes for each data type to find
dt_search_prefixes = {}
Expand Down Expand Up @@ -979,7 +979,7 @@ def parse_var_list(config, time_info=None, data_type=None, met_tool=None,
# check if number of levels for each field type matches
n_levels = len(field_info_list[0]['levels'])
if len(data_types) > 1:
if (n_levels != len(field_info_list[1]['levels'])):
if n_levels != len(field_info_list[1]['levels']):
continue

# if requested, put all field levels in a single item
Expand Down Expand Up @@ -1064,7 +1064,22 @@ def parse_var_list(config, time_info=None, data_type=None, met_tool=None,
'''
return sorted(var_list, key=lambda x: x['index'])


def _find_var_name_indices(config, data_types, met_tool=None):
"""!Get list of indices used in _VAR<n>_ config variables. Data type
determines prefix of variable name to find. If FCST or OBS is included
in data type list, then BOTH keyword is also searched. If specified,
wrapper-specific variables are searched, e.g. FCST_GRID_STAT_VAR<n>_*.
Variables that end with NAME, INPUT_FIELD_NAME, or FIELD_NAME are used to
gather indices.
@param config METplusConfig object to read
@param data_types list of prefixes of config variables that describe the
type of data e.g. FCST or OBS.
@param met_tool (optional) name of wrapper to search for wrapper-specific
variables, e.g. *_GRID_STAT_VAR<n>_*.
@returns list of integers for all matching config variables
"""
data_type_regex = f"{'|'.join(data_types)}"

# if data_types includes FCST or OBS, also search for BOTH
Expand All @@ -1080,10 +1095,11 @@ def _find_var_name_indices(config, data_types, met_tool=None):
regex_string += r"_VAR(\d+)_(NAME|INPUT_FIELD_NAME|FIELD_NAME)"

# find all <data_type>_VAR<n>_NAME keys in the conf files
return find_indices_in_config_section(regex_string,
config,
index_index=2,
id_index=1)
indices = find_indices_in_config_section(regex_string,
config,
index_index=2,
id_index=1).keys()
return [int(index) for index in indices]


def _format_var_items(field_configs, time_info=None):
Expand Down Expand Up @@ -1210,7 +1226,7 @@ def get_field_config_variables(config, index, search_prefixes):
in RegridDataPlane wrapper.
@param config METplusConfig object to search
@param index of field (VAR<n>) to find
@param index integer <n> of field (VAR<n>) to find
@param search_prefixes list of valid prefixes to search for variables
in the config, i.e. FCST_VAR1_ or OBS_GRID_STAT_VAR2_
@returns dictionary containing a config variable name to be used for
Expand Down
2 changes: 2 additions & 0 deletions metplus/util/config_util.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -212,6 +212,8 @@ def _sub_var_info(var_info, time_info):
out_value.append(do_string_sub(item,
skip_missing_tags=True,
**time_info))
elif isinstance(value, int):
out_value = value
else:
out_value = do_string_sub(value,
skip_missing_tags=True,
Expand Down
6 changes: 2 additions & 4 deletions metplus/util/string_manip.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -534,11 +534,9 @@ def find_indices_in_config_section(regex, config, sec='config',
@param index_index 1 based number that is the regex match index for the
index number (default is 1)
@param id_index 1 based number that is the regex match index for the
identifier. Defaults to None which does not extract an indentifier
number and the first match is used as an identifier
identifier. Defaults to None which does not extract an identifier
@returns dictionary where keys are the index number and the value is a
list of identifiers (if noID=True) or a list containing None
list of identifiers (if id_index=None) or a list containing None
"""
# regex expression must have 2 () items and the 2nd item must be the index
all_conf = config.keys(sec)
Expand Down

0 comments on commit c19aebc

Please sign in to comment.