Skip to content

Commit

Permalink
Feature #511 lists in command line single config overrides (#2815)
Browse files Browse the repository at this point in the history
* exclude metplus/scripts dir from PyCharm project to prevent incorrect duplicate code warnings with files that are sym linked

* to expand on #2772, updating instructions to include note to set tmp directory for apptainer to prevent issues pulling large images that require a lot of temp space

* per #511, add unit test for expected behavior to support comma-separated lists in a command line single config override that should fail until fix is made

* add a test to ensure that the -c argument is properly ignored since it is can be used in old use cases that were created when the argument was required

* per #511, add support for command line single config overrides to include values that are lists. Simplify logic to parse arguments to strip out -c/--config/-config arguments and skip check/error if argument is invalid because it is already handled in the metplus_config setup step that parses the arguments

* added unit test to ensure that an invalid command line argument causes the appropriate failure from run_metplus.py
  • Loading branch information
georgemccabe authored Dec 12, 2024
1 parent 75476cd commit dc75ada
Show file tree
Hide file tree
Showing 4 changed files with 27 additions and 19 deletions.
1 change: 1 addition & 0 deletions .idea/METplus.iml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

14 changes: 13 additions & 1 deletion docs/Users_Guide/getting_started.rst
Original file line number Diff line number Diff line change
Expand Up @@ -463,10 +463,22 @@ METplus in Apptainer
Apptainer (formerly Singularity) can also be used to run METplus.
Images can be pulled from DockerHub using Apptainer commands.

If running on the NCAR Casper cluster, be sure to first load the Apptainer module via::
If running on the NCAR Casper cluster,
be sure to first load the Apptainer module via::

module load apptainer

It is recommended to set the **APPTAINER_TMPDIR** environment variable to a
directory where the user has write permissions to prevent issues with temporary
files exceeding the size of the /tmp directory.
See the
`Apptainer documentation <https://apptainer.org/docs/user/1.0/build_env.html#temporary-folders>`_
for more information.

Example::

export APPTAINER_TMP_DIR=/d1/${USER}/apptainer_tmp

Navigate to a working directory and pull an image from DockerHub, e.g.::

apptainer pull docker://dtcenter/metplus:5.1-latest
Expand Down
10 changes: 10 additions & 0 deletions internal/tests/pytests/run_metplus/test_run_metplus.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@
NEW_OUTPUT_BASE = os.path.join(TEST_OUTPUT_DIR, 'run_metplus')
OUTPUT_BASE_OVERRIDE = f"config.OUTPUT_BASE={NEW_OUTPUT_BASE}"

# test that a list of values specified in a command line config override
# will no longer cause an error in the run. The list can be specified without
# quotes if no spaces are present or with quotes if spaces are present
LIST_CONFIG_OVERRIDE_1 = 'config.LEAD_SEQ=3H,6H'
LIST_CONFIG_OVERRIDE_2 = 'config.LEAD_SEQ="3H, 6H"'

@pytest.mark.run_metplus
def test_run_metplus_exists():
"""! Check that run_metplus.py script exists """
Expand All @@ -31,6 +37,10 @@ def test_run_metplus_exists():
([RUN_METPLUS], 2),
([RUN_METPLUS, EXAMPLE_CONF], 2),
([RUN_METPLUS, EXAMPLE_CONF, MINIMUM_CONF, OUTPUT_BASE_OVERRIDE], 0),
([RUN_METPLUS, '-c', EXAMPLE_CONF, MINIMUM_CONF, OUTPUT_BASE_OVERRIDE], 0),
([RUN_METPLUS, EXAMPLE_CONF, MINIMUM_CONF, LIST_CONFIG_OVERRIDE_1], 0),
([RUN_METPLUS, EXAMPLE_CONF, MINIMUM_CONF, LIST_CONFIG_OVERRIDE_2], 0),
([RUN_METPLUS, EXAMPLE_CONF, MINIMUM_CONF, '--fake-arg'], 1),
]
)
@pytest.mark.run_metplus
Expand Down
21 changes: 3 additions & 18 deletions ush/run_metplus.py
Original file line number Diff line number Diff line change
Expand Up @@ -78,24 +78,9 @@ def get_config_inputs_from_command_line():
if any(arg in sys.argv for arg in help_args):
usage()

# pull out command line arguments
config_inputs = []
for arg in sys.argv[1:]:
if arg.startswith('-'):
# ignore -c and --config since they are now optional
if arg == '-c' or arg == '--config' or arg == '-config':
continue

# error/exit if an argument that is not supported was used
print('ERROR: Invalid argument: %s.' % arg)
usage()

# split up comma separated lists into individual items
# and add each to list of arguments
# NOTE: to support lists in a config variable override,
# this logic will have to be enhanced
# i.e. config.PROCESS_LIST=PCPCombine,GridStat
config_inputs.extend(arg.split(','))
# pull out command line arguments, removing deprecated config arguments
config_args = ('-c', '--config', '-config')
config_inputs = [arg for arg in sys.argv[1:] if arg not in config_args]

# if no valid config_inputs were found, print usage and exit
if not config_inputs:
Expand Down

0 comments on commit dc75ada

Please sign in to comment.