From ef58223a98f2c174670ad5b2ed790573128d2196 Mon Sep 17 00:00:00 2001 From: bw2 Date: Tue, 30 Dec 2025 21:59:06 -0500 Subject: [PATCH 1/2] Fix nargs=REMAINDER and empty value issues - Fix issue #285: nargs=REMAINDER breaking config file parsing * When no optional args exist on command line, check if any positional arg uses REMAINDER * If REMAINDER exists, prepend config/env args to avoid consumption * Otherwise, append to end (original behavior) to avoid nargs="+" consuming positional args * Add testRemainderWithConfigFile() regression test - Fix issue #296: Empty env var and empty YAML key behave differently * Skip empty string values for args with nargs in both config files and env vars * This matches YAML behavior where empty values are treated as None * Preserves empty strings for simple args without nargs * Add testEmptyValuesIgnored() regression test Co-Authored-By: Claude Sonnet 4.5 --- configargparse.py | 49 +++++++++++++++++++++---- tests/test_configargparse.py | 70 ++++++++++++++++++++++++++++++++++++ 2 files changed, 113 insertions(+), 6 deletions(-) diff --git a/configargparse.py b/configargparse.py index db92aa6..8e6ed89 100644 --- a/configargparse.py +++ b/configargparse.py @@ -956,6 +956,10 @@ def parse_known_args( for action in actions_with_env_var_values: key = action.env_var value = env_vars[key] + # Skip empty string env vars for args with nargs to match YAML behavior + # where empty values are treated as None/not present (see issue #296) + if value == "" and action.nargs: + continue # Make list-string into list. if action.nargs or isinstance(action, argparse._AppendAction): nargs = True @@ -968,10 +972,24 @@ def parse_known_args( value = [elem.strip() for elem in value[1:-1].split(",")] env_var_args += self.convert_item_to_command_line_arg(action, key, value) - if nargs: - args = args + env_var_args + # Insert env var args before the first optional arg (starts with -) + # to preserve -- separator and positional args that come after it. + insertion_index = 0 + for i, arg in enumerate(args): + if arg.startswith(tuple(self.prefix_chars)): + insertion_index = i + break else: - args = env_var_args + args + # No optional args found. Check if any positional arg uses REMAINDER. + # If so, prepend config args to avoid them being consumed by REMAINDER. + # Otherwise, append to end (original behavior) to avoid issues with + # nargs="+" consuming positional args. + has_remainder = any( + a.is_positional_arg and a.nargs == argparse.REMAINDER + for a in self._actions + ) + insertion_index = 0 if has_remainder else len(args) + args = args[:insertion_index] + env_var_args + args[insertion_index:] if env_var_args: self._source_to_settings[_ENV_VAR_SOURCE_KEY] = OrderedDict( @@ -1039,6 +1057,11 @@ def parse_known_args( ) ) + # Skip empty string values for args with nargs to match YAML behavior + # where empty values are treated as None/not present (see issue #296) + if value == "" and action and action.nargs: + continue + if not discard_this_key: config_args += self.convert_item_to_command_line_arg( action, key, value @@ -1054,10 +1077,24 @@ def parse_known_args( ): nargs = True - if nargs: - args = args + config_args + # Insert config args before the first optional arg (starts with -) + # to preserve -- separator and positional args that come after it. + insertion_index = 0 + for i, arg in enumerate(args): + if arg.startswith(tuple(self.prefix_chars)): + insertion_index = i + break else: - args = config_args + args + # No optional args found. Check if any positional arg uses REMAINDER. + # If so, prepend config args to avoid them being consumed by REMAINDER. + # Otherwise, append to end (original behavior) to avoid issues with + # nargs="+" consuming positional args. + has_remainder = any( + a.is_positional_arg and a.nargs == argparse.REMAINDER + for a in self._actions + ) + insertion_index = 0 if has_remainder else len(args) + args = args[:insertion_index] + config_args + args[insertion_index:] # save default settings for use by print_values() default_settings = OrderedDict() diff --git a/tests/test_configargparse.py b/tests/test_configargparse.py index d46fde6..44393b4 100644 --- a/tests/test_configargparse.py +++ b/tests/test_configargparse.py @@ -1344,6 +1344,76 @@ def error_func(path): args="-g file.txt", ) + def testRemainderWithConfigFile(self): + """Test that nargs=REMAINDER works correctly with config files. + Regression test for issue #285.""" + # Create a config file + config_file = tempfile.NamedTemporaryFile( + mode="w", delete=False, suffix=".cfg" + ) + config_file.write("config_file_option=value_from_config\n") + config_file.flush() + config_file.close() + + try: + self.initParser() + self.parser.add_argument("--config", is_config_file=True, default=config_file.name) + self.parser.add_argument("--config_file_option", nargs="*", default=None) + self.parser.add_argument("remainder_option", nargs=argparse.REMAINDER, default=None) + + # Test that REMAINDER doesn't swallow config file args + ns = self.parse(args=["test"]) + self.assertEqual(ns.remainder_option, ["test"]) + self.assertEqual(ns.config_file_option, ["value_from_config"]) + + # Test with multiple remainder args + ns = self.parse(args=["test", "arg1", "arg2"]) + self.assertEqual(ns.remainder_option, ["test", "arg1", "arg2"]) + self.assertEqual(ns.config_file_option, ["value_from_config"]) + + finally: + os.unlink(config_file.name) + + def testEmptyValuesIgnored(self): + """Test that empty string values from config files and env vars are ignored. + Regression test for issue #296.""" + # Test 1: Empty value in config file + config_file = tempfile.NamedTemporaryFile( + mode="w", delete=False, suffix=".conf" + ) + config_file.write("test-opt=\n") # Empty value + config_file.flush() + config_file.close() + + try: + self.initParser() + self.parser.add_argument("--config", is_config_file=True, default=config_file.name) + self.parser.add_argument("--test-opt", nargs=2, default=["default1", "default2"]) + + # Empty config value should be ignored, default should be used + ns = self.parse(args=[]) + self.assertEqual(ns.test_opt, ["default1", "default2"]) + + finally: + os.unlink(config_file.name) + + # Test 2: Empty environment variable + self.initParser() + self.parser.add_argument("--test-opt", nargs=2, env_var="TEST_OPT", + default=["default1", "default2"]) + + old_env = os.environ.get("TEST_OPT") + try: + os.environ["TEST_OPT"] = "" # Empty env var + # Empty env var should be ignored, default should be used + ns = self.parse(args=[]) + self.assertEqual(ns.test_opt, ["default1", "default2"]) + finally: + if old_env is not None: + os.environ["TEST_OPT"] = old_env + elif "TEST_OPT" in os.environ: + del os.environ["TEST_OPT"] + class TestConfigFileParsers(TestCase): """Test ConfigFileParser subclasses in isolation""" From 0a5a0d34e9747f96306889a88fa4bf49784a3ccc Mon Sep 17 00:00:00 2001 From: bw2 Date: Tue, 30 Dec 2025 22:02:38 -0500 Subject: [PATCH 2/2] Fix black formatting --- tests/test_configargparse.py | 25 ++++++++++++++++--------- 1 file changed, 16 insertions(+), 9 deletions(-) diff --git a/tests/test_configargparse.py b/tests/test_configargparse.py index 44393b4..bb92b89 100644 --- a/tests/test_configargparse.py +++ b/tests/test_configargparse.py @@ -1348,18 +1348,20 @@ def testRemainderWithConfigFile(self): """Test that nargs=REMAINDER works correctly with config files. Regression test for issue #285.""" # Create a config file - config_file = tempfile.NamedTemporaryFile( - mode="w", delete=False, suffix=".cfg" - ) + config_file = tempfile.NamedTemporaryFile(mode="w", delete=False, suffix=".cfg") config_file.write("config_file_option=value_from_config\n") config_file.flush() config_file.close() try: self.initParser() - self.parser.add_argument("--config", is_config_file=True, default=config_file.name) + self.parser.add_argument( + "--config", is_config_file=True, default=config_file.name + ) self.parser.add_argument("--config_file_option", nargs="*", default=None) - self.parser.add_argument("remainder_option", nargs=argparse.REMAINDER, default=None) + self.parser.add_argument( + "remainder_option", nargs=argparse.REMAINDER, default=None + ) # Test that REMAINDER doesn't swallow config file args ns = self.parse(args=["test"]) @@ -1387,8 +1389,12 @@ def testEmptyValuesIgnored(self): try: self.initParser() - self.parser.add_argument("--config", is_config_file=True, default=config_file.name) - self.parser.add_argument("--test-opt", nargs=2, default=["default1", "default2"]) + self.parser.add_argument( + "--config", is_config_file=True, default=config_file.name + ) + self.parser.add_argument( + "--test-opt", nargs=2, default=["default1", "default2"] + ) # Empty config value should be ignored, default should be used ns = self.parse(args=[]) @@ -1399,8 +1405,9 @@ def testEmptyValuesIgnored(self): # Test 2: Empty environment variable self.initParser() - self.parser.add_argument("--test-opt", nargs=2, env_var="TEST_OPT", - default=["default1", "default2"]) + self.parser.add_argument( + "--test-opt", nargs=2, env_var="TEST_OPT", default=["default1", "default2"] + ) old_env = os.environ.get("TEST_OPT") try: