Skip to content

Commit

Permalink
Bugfix #2179 develop TCPairs fix -diag argument (#2187)
Browse files Browse the repository at this point in the history
  • Loading branch information
georgemccabe authored Jun 5, 2023
1 parent d4994b1 commit 4d28b4d
Show file tree
Hide file tree
Showing 5 changed files with 85 additions and 63 deletions.
6 changes: 2 additions & 4 deletions internal/tests/pytests/util/config/test_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def test_getstr(metplus_config, input_value, default, result):
('/some/test/dir', None, '/some/test/dir'),
('/some//test/dir', None, '/some/test/dir'),
('/path/to', None, 'ValueError'),
(None, None, 'NoOptionError'),
(None, None, ''),
(None, '/default/path', '/default/path'),
]
Expand All @@ -83,11 +83,9 @@ def test_getdir(metplus_config, input_value, default, result):
if input_value is not None:
conf.set('config', 'TEST_GETDIR', input_value)

# catch NoOptionError exception and pass test if default is None
# catch ValueError and pass test if value is invalid and no default
try:
assert result == conf.getdir('TEST_GETDIR', default=default)
except NoOptionError:
assert result == 'NoOptionError'
except ValueError:
assert result == 'ValueError'

Expand Down
21 changes: 21 additions & 0 deletions internal/tests/pytests/wrappers/tc_pairs/test_tc_pairs_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,6 +78,7 @@ def test_read_storm_info(metplus_config, config_overrides, isOK):
wrapper = TCPairsWrapper(config)
assert wrapper.isOK == isOK


@pytest.mark.parametrize(
'storm_id,basin,cyclone', [
('AL092011', 'al', '09'),
Expand Down Expand Up @@ -540,6 +541,23 @@ def test_tc_pairs_storm_id_lists(metplus_config, config_overrides,
'field_source = "GFS_0p50";match_to_track = ["OFCL"]'
';diag_name = ["MY_NAME2"];}];'
)}),
# -diag arguments with multiple models
('VALID', {
'MODEL': 'MYNN, H19C',
'TC_PAIRS_DIAG_TEMPLATE1': '/some/path/{valid?fmt=%Y%m%d%H}_{model}.dat',
'TC_PAIRS_DIAG_SOURCE1': 'TCDIAG',
'TC_PAIRS_DIAG_TEMPLATE2': '/some/path/rt_{valid?fmt=%Y%m%d%H}.dat',
'TC_PAIRS_DIAG_SOURCE2': 'LSDIAG_RT',
},
{'DIAG_ARG': ('-diag TCDIAG /some/path/2014121318_MYNN.dat /some/path/2014121318_H19C.dat '
'-diag LSDIAG_RT /some/path/rt_2014121318.dat'),
'METPLUS_MODEL': 'model = ["MYNN", "H19C"];'}),
# -diag arguments with wildcard model
('VALID', {
'TC_PAIRS_DIAG_TEMPLATE1': 'bmlq2014123118.{model}.0104',
'TC_PAIRS_DIAG_SOURCE1': 'TCDIAG',
},
{'DIAG_ARG': '-diag TCDIAG <BDECK_DIR>/bmlq2014123118.gfso.0104',}),
]
)
@pytest.mark.wrapper
Expand All @@ -556,6 +574,8 @@ def test_tc_pairs_run(metplus_config, loop_by, config_overrides,

config.set('config', 'TC_PAIRS_BDECK_INPUT_DIR', bdeck_dir)
config.set('config', 'TC_PAIRS_ADECK_INPUT_DIR', adeck_dir)
if 'DIAG_ARG' in env_var_values and 'TC_PAIRS_DIAG_DIR1' not in config_overrides:
config.set('config', 'TC_PAIRS_DIAG_DIR1', bdeck_dir)

# set config variable overrides
for key, value in config_overrides.items():
Expand Down Expand Up @@ -589,6 +609,7 @@ def test_tc_pairs_run(metplus_config, loop_by, config_overrides,
diag_arg = ''
if 'DIAG_ARG' in env_var_values:
diag_arg = f" {env_var_values['DIAG_ARG']}"
diag_arg = diag_arg.replace('<BDECK_DIR>', bdeck_dir)

expected_cmds = [(f"{app_path} {verbosity} "
f"-bdeck {bdeck_dir}/bmlq2014123118.gfso.0104 "
Expand Down
20 changes: 8 additions & 12 deletions metplus/util/config_metplus.py
Original file line number Diff line number Diff line change
Expand Up @@ -530,10 +530,12 @@ def getraw(self, sec, opt, default='', count=0, sub_vars=True):
config, dir, and os environment)
returns raw string, preserving {valid?fmt=%Y} blocks
@param sec: Section in the conf file to look for variable
@param opt: Variable to interpret
@param default: Default value to use if config is not set
@param count: Counter used to stop recursion to prevent infinite
@param sec Section in the conf file to look for variable
@param opt Variable to interpret
@param default Default value to use if config is not set
@param count Counter used to stop recursion to prevent infinite
@param sub_vars If False, skip string template substitution,
defaults to True
@returns Raw string or empty string if function calls itself too
many times
"""
Expand Down Expand Up @@ -640,17 +642,11 @@ def getexe(self, exe_name):
self.set('config', exe_name, full_exe_path)
return full_exe_path

def getdir(self, dir_name, default=None, morevars=None,taskvars=None,
must_exist=False):
def getdir(self, dir_name, default=None, must_exist=False):
"""! Wraps produtil getdir and reports an error if
it is set to /path/to
"""
try:
dir_path = super().getstr('config', dir_name, default=None,
morevars=morevars, taskvars=taskvars)
except NoOptionError:
self.check_default('config', dir_name, default)
dir_path = default
dir_path = self.getraw('config', dir_name, default=default)

if '/path/to' in dir_path:
raise ValueError(f"{dir_name} cannot be set to "
Expand Down
18 changes: 9 additions & 9 deletions metplus/wrappers/command_builder.py
Original file line number Diff line number Diff line change
Expand Up @@ -566,13 +566,11 @@ def find_exact_file(self, level, data_type, time_info, mandatory=True,
input_must_exist = self.c_dict.get('INPUT_MUST_EXIST', True)

for template in template_list:
# perform string substitution
filename = do_string_sub(template,
level=level,
**time_info)

# build full path with data directory and filename
full_path = os.path.join(data_dir, filename)
# perform string substitution on directory and template
full_template = os.path.join(data_dir, template)
full_path = do_string_sub(full_template,
level=level,
**time_info)

if os.path.sep not in full_path:
self.logger.debug(f"{full_path} is not a file path. "
Expand Down Expand Up @@ -690,6 +688,9 @@ def find_file_in_window(self, level, data_type, time_info, mandatory=True,
self.log_error('Must set INPUT_DIR if looking for files within a time window')
return None

# substitute any filename template tags that may be in _INPUT_DIR
data_dir = do_string_sub(data_dir, **time_info)

# step through all files under input directory in sorted order
for dirpath, _, all_files in os.walk(data_dir):
for filename in sorted(all_files):
Expand Down Expand Up @@ -941,8 +942,7 @@ def find_and_check_output_file(self, time_info=None,

# substitute time info if provided
if time_info:
output_path = do_string_sub(output_path,
**time_info)
output_path = do_string_sub(output_path, **time_info)

# replace wildcard character * with all
output_path = output_path.replace('*', 'all')
Expand Down
83 changes: 45 additions & 38 deletions metplus/wrappers/tc_pairs_wrapper.py
Original file line number Diff line number Diff line change
Expand Up @@ -474,6 +474,10 @@ def _handle_diag(self, c_dict):
source = (
self.config.getraw('config', f'TC_PAIRS_DIAG_SOURCE{idx}')
)
if not source:
self.log_error(f'TC_PAIRS_DIAG_SOURCE{idx} must be set if '
f'TC_PAIRS_DIAG_TEMPLATE{idx} is set')
continue
diag_info = {
'template': template,
'source': source,
Expand Down Expand Up @@ -658,6 +662,10 @@ def process_data(self, basin, cyclone, time_info):
if current_basin is None or current_cyclone is None:
continue

time_storm_info = time_info.copy()
time_storm_info['basin'] = current_basin
time_storm_info['cyclone'] = current_cyclone

# create lists for deck files, put bdeck in list so it can
# be handled the same as a and e for reformatting even though
# it will always be size 1
Expand All @@ -667,14 +675,10 @@ def process_data(self, basin, cyclone, time_info):

# get adeck files
if self.c_dict['GET_ADECK']:
adeck_list = self.find_a_or_e_deck_files('A', current_basin,
current_cyclone,
time_info)
adeck_list = self.find_a_or_e_deck_files('A', time_storm_info)
# get edeck files
if self.c_dict['GET_EDECK']:
edeck_list = self.find_a_or_e_deck_files('E', current_basin,
current_cyclone,
time_info)
edeck_list = self.find_a_or_e_deck_files('E', time_storm_info)

if not adeck_list and not edeck_list:
self.log_error('Could not find any corresponding '
Expand All @@ -693,20 +697,16 @@ def process_data(self, basin, cyclone, time_info):
if edeck_list:
self.args.append(f"-edeck {' '.join(edeck_list)}")

# change wildcard basin/cyclone to 'all' for output filename
if current_basin == self.WILDCARDS['basin']:
current_basin = 'all'
if current_cyclone == self.WILDCARDS['cyclone']:
current_cyclone = 'all'

time_storm_info = time_info.copy()
time_storm_info['basin'] = current_basin
time_storm_info['cyclone'] = current_cyclone

# find -diag file if requested
if not self._get_diag_file(time_storm_info):
return []

# change wildcard basin/cyclone to 'all' for output filename
if current_basin == self.WILDCARDS['basin']:
time_storm_info['basin'] = 'all'
if current_cyclone == self.WILDCARDS['cyclone']:
time_storm_info['cyclone'] = 'all'

if not self.find_and_check_output_file(time_info=time_storm_info,
check_extension='.tcst'):
return []
Expand Down Expand Up @@ -811,22 +811,18 @@ def _get_basin_cyclone_from_bdeck(self, bdeck_file, wildcard_used,

return current_basin, current_cyclone

def find_a_or_e_deck_files(self, deck, basin, cyclone, time_info):
def find_a_or_e_deck_files(self, deck, time_info):
"""!Find ADECK or EDECK files that correspond to the BDECk file found
@param deck type of deck (A or E)
@param basin region of storm from config
@param cyclone ID number of cyclone from config
@param time_info dictionary with timing info for current run
@param time_info dictionary with timing/storm info for current run
"""
deck_list = []
template = os.path.join(self.c_dict[deck+'DECK_DIR'],
self.c_dict[deck+'DECK_TEMPLATE'])

# get matching adeck wildcard expression for first model
deck_expr = do_string_sub(template,
basin=basin,
cyclone=cyclone,
model=self.c_dict['MODEL_LIST'][0],
**time_info)

Expand Down Expand Up @@ -899,16 +895,18 @@ def get_command(self):
f" -out {output_path}")
return cmd

def read_modify_write_file(self, in_csvfile, storm_month, missing_values,
@staticmethod
def read_modify_write_file(in_csvfile, storm_month, missing_values,
out_csvfile):
"""! Reads, modifies and writes file
Args:
@param in_csvfile input csv file that is being parsed
@param storm_month The storm month
@param missing_values a tuple where (MISSING_VAL_TO_REPLACE,
MISSING_VAL)
@param out_csvfile the output csv file
@param logger the log where logging is directed
"""!Reads CSV file, reformat file by adding the month to the 2nd
column storm number, delete the 3rd column, replace missing values,
and write a new CSV file with the modified content.
@param in_csvfile input csv file that is being parsed
@param storm_month storm month to prepend to storm number
@param missing_values tuple containing a missing data value to find in
the columns and the value to replace it with, e.g. (-9, -9999)
@param out_csvfile the output csv file
"""
# create output directory if it does not exist
mkdir_p(os.path.dirname(out_csvfile))
Expand Down Expand Up @@ -943,15 +941,14 @@ def read_modify_write_file(self, in_csvfile, storm_month, missing_values,
continue
# Replace MISSING_VAL_TO_REPLACE=missing_values[0] with
# MISSING_VAL=missing_values[1]
elif item.strip() == missing_values[0]:
if item.strip() == missing_values[0]:
item = " " + missing_values[1]
# Create a new row to write
row_list.append(item)

# Write the modified file
writer.writerow(row_list)

csvfile.close()
out_file.close()

def _read_all_files(self, input_dict):
Expand Down Expand Up @@ -1005,16 +1002,26 @@ def _get_diag_file(self, time_info):
"""
if not self.c_dict.get('DIAG_INFO_LIST'):
return True
self.log_error(self.c_dict.get('DIAG_INFO_LIST'))

time_info_copy = time_info.copy()
for diag_info in self.c_dict.get('DIAG_INFO_LIST'):
self.c_dict['DIAG_INPUT_TEMPLATE'] = diag_info['template']
filepaths = self.find_data(time_info, data_type='DIAG',
return_list=True)
if not filepaths:
all_files = []
for model in self.c_dict['MODEL_LIST']:
time_info_copy['model'] = model
filepaths = self.find_data(time_info_copy, data_type='DIAG',
return_list=True)
if filepaths:
all_files.extend(filepaths)

if not all_files:
self.log_error('Could not get -diag files')
return False

arg = f"-diag {diag_info['source']} {' '.join(filepaths)}"
# remove duplicate files
all_files = list(set(all_files))

arg = f"-diag {diag_info['source']} {' '.join(all_files)}"
self.args.append(arg)

return True
Expand Down

0 comments on commit 4d28b4d

Please sign in to comment.