Fix settings being overwritten when using the same dest attribute

This commit is contained in:
Timmy Welch 2023-12-16 16:51:00 -08:00
parent 9ffefb3e21
commit 4c385667e8
2 changed files with 27 additions and 13 deletions

View File

@ -107,6 +107,9 @@ class Setting:
exclusive: bool = False, exclusive: bool = False,
): ):
""" """
Attributes:
setting_name: This is the name used to retrieve this Setting object from a `Config` Definitions dictionary.
This only differs from dest when a custom dest is given
Args: Args:
*names: Passed directly to argparse *names: Passed directly to argparse
@ -119,7 +122,8 @@ class Setting:
required: Passed directly to argparse required: Passed directly to argparse
help: Passed directly to argparse help: Passed directly to argparse
metavar: Passed directly to argparse, defaults to `dest` upper-cased metavar: Passed directly to argparse, defaults to `dest` upper-cased
dest: This is the name used to retrieve the value from a `Config` object as a dictionary dest: This is the name used to retrieve the value from a `Config` object as a dictionary.
Default to `setting_name`.
display_name: This is not used by settngs. This is a human-readable name to be used when generating a GUI. display_name: This is not used by settngs. This is a human-readable name to be used when generating a GUI.
Defaults to `dest`. Defaults to `dest`.
cmdline: If this setting can be set via the commandline cmdline: If this setting can be set via the commandline
@ -133,7 +137,7 @@ class Setting:
raise ValueError('names must be specified') raise ValueError('names must be specified')
# We prefix the destination name used by argparse so that there are no conflicts # We prefix the destination name used by argparse so that there are no conflicts
# Argument names will still cause an exception if there is a conflict e.g. if '-f' is defined twice # Argument names will still cause an exception if there is a conflict e.g. if '-f' is defined twice
self.internal_name, dest, self.flag = self.get_dest(group, names, dest) self.internal_name, setting_name, dest, self.flag = self.get_dest(group, names, dest)
args: Sequence[str] = names args: Sequence[str] = names
# We then also set the metavar so that '--config' in the group runtime shows as 'CONFIG' instead of 'RUNTIME_CONFIG' # We then also set the metavar so that '--config' in the group runtime shows as 'CONFIG' instead of 'RUNTIME_CONFIG'
@ -155,6 +159,7 @@ class Setting:
self.help = help self.help = help
self.metavar = metavar self.metavar = metavar
self.dest = dest self.dest = dest
self.setting_name = setting_name
self.cmdline = cmdline self.cmdline = cmdline
self.file = file self.file = file
self.argparse_args = args self.argparse_args = args
@ -228,28 +233,30 @@ class Setting:
return None return None
return 'Any' return 'Any'
def get_dest(self, prefix: str, names: Sequence[str], dest: str | None) -> tuple[str, str, bool]: def get_dest(self, prefix: str, names: Sequence[str], dest: str | None) -> tuple[str, str, str, bool]:
dest_name = None setting_name = None
flag = False flag = False
prefix = sanitize_name(prefix) prefix = sanitize_name(prefix)
for n in names: for n in names:
if n.startswith('--'): if n.startswith('--'):
flag = True flag = True
dest_name = sanitize_name(n) setting_name = sanitize_name(n)
break break
if n.startswith('-'): if n.startswith('-'):
flag = True flag = True
if dest_name is None: if setting_name is None:
dest_name = names[0] setting_name = names[0]
if dest: if dest:
dest_name = dest dest_name = dest
else:
dest_name = setting_name
if not dest_name.isidentifier(): if not dest_name.isidentifier():
raise Exception(f'Cannot use {dest_name} in a namespace') raise Exception(f'Cannot use {dest_name} in a namespace')
internal_name = f'{prefix}__{dest_name}'.lstrip('_') internal_name = f'{prefix}__{dest_name}'.lstrip('_')
return internal_name, dest_name, flag return internal_name, setting_name, dest_name, flag
def filter_argparse_kwargs(self) -> dict[str, Any]: def filter_argparse_kwargs(self) -> dict[str, Any]:
return {k: v for k, v in self.argparse_kwargs.items() if v is not None} return {k: v for k, v in self.argparse_kwargs.items() if v is not None}
@ -708,7 +715,7 @@ class Manager:
"""Passes all arguments through to `Setting`, `group` and `exclusive` are already set""" """Passes all arguments through to `Setting`, `group` and `exclusive` are already set"""
setting = Setting(*args, **kwargs, group=self.current_group_name, exclusive=self.exclusive_group) setting = Setting(*args, **kwargs, group=self.current_group_name, exclusive=self.exclusive_group)
self.definitions[self.current_group_name].v[setting.dest] = setting self.definitions[self.current_group_name].v[setting.setting_name] = setting
def add_group(self, name: str, group: Callable[[Manager], None], exclusive_group: bool = False) -> None: def add_group(self, name: str, group: Callable[[Manager], None], exclusive_group: bool = False) -> None:
""" """

View File

@ -77,6 +77,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test_setting', # dest is calculated by Setting and is not used by argparse
'dest': 'test_setting', # dest is calculated by Setting and is not used by argparse 'dest': 'test_setting', # dest is calculated by Setting and is not used by argparse
'display_name': 'test_setting', # defaults to dest 'display_name': 'test_setting', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -84,7 +85,7 @@ success = [
'flag': True, 'flag': True,
'group': 'tst', 'group': 'tst',
'help': None, 'help': None,
'internal_name': 'tst__test_setting', # Should almost always be "{group}_{dest}" 'internal_name': 'tst__test_setting', # Should almost always be "{group}__{dest}"
'metavar': 'TEST_SETTING', # Set manually so argparse doesn't use TST_TEST 'metavar': 'TEST_SETTING', # Set manually so argparse doesn't use TST_TEST
'nargs': None, 'nargs': None,
'required': None, 'required': None,
@ -118,6 +119,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test', # setting_name is calculated by Setting and is not used by argparse
'dest': 'testing', # dest is calculated by Setting and is not used by argparse 'dest': 'testing', # dest is calculated by Setting and is not used by argparse
'display_name': 'testing', # defaults to dest 'display_name': 'testing', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -125,7 +127,7 @@ success = [
'flag': True, 'flag': True,
'group': 'tst', 'group': 'tst',
'help': None, 'help': None,
'internal_name': 'tst__testing', # Should almost always be "{group}_{dest}" 'internal_name': 'tst__testing', # Should almost always be "{group}__{dest}"
'metavar': 'TESTING', # Set manually so argparse doesn't use TST_TEST 'metavar': 'TESTING', # Set manually so argparse doesn't use TST_TEST
'nargs': None, 'nargs': None,
'required': None, 'required': None,
@ -158,6 +160,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test', # dest is calculated by Setting and is not used by argparse
'dest': 'test', # dest is calculated by Setting and is not used by argparse 'dest': 'test', # dest is calculated by Setting and is not used by argparse
'display_name': 'test', # defaults to dest 'display_name': 'test', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -165,7 +168,7 @@ success = [
'flag': True, 'flag': True,
'group': 'tst', 'group': 'tst',
'help': None, 'help': None,
'internal_name': 'tst__test', # Should almost always be "{group}_{dest}" 'internal_name': 'tst__test', # Should almost always be "{group}__{dest}"
'metavar': 'TEST', # Set manually so argparse doesn't use TST_TEST 'metavar': 'TEST', # Set manually so argparse doesn't use TST_TEST
'nargs': None, 'nargs': None,
'required': None, 'required': None,
@ -199,6 +202,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test', # dest is calculated by Setting and is not used by argparse
'dest': 'test', # dest is calculated by Setting and is not used by argparse 'dest': 'test', # dest is calculated by Setting and is not used by argparse
'display_name': 'test', # defaults to dest 'display_name': 'test', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -206,7 +210,7 @@ success = [
'flag': True, 'flag': True,
'group': 'tst', 'group': 'tst',
'help': None, 'help': None,
'internal_name': 'tst__test', # Should almost always be "{group}_{dest}" 'internal_name': 'tst__test', # Should almost always be "{group}__{dest}"
'metavar': None, # store_true does not get a metavar 'metavar': None, # store_true does not get a metavar
'nargs': None, 'nargs': None,
'required': None, 'required': None,
@ -239,6 +243,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test',
'dest': 'test', 'dest': 'test',
'display_name': 'test', # defaults to dest 'display_name': 'test', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -279,6 +284,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test',
'dest': 'test', 'dest': 'test',
'display_name': 'test', # defaults to dest 'display_name': 'test', # defaults to dest
'exclusive': False, 'exclusive': False,
@ -317,6 +323,7 @@ success = [
'cmdline': True, 'cmdline': True,
'const': None, 'const': None,
'default': None, 'default': None,
'setting_name': 'test',
'dest': 'test', 'dest': 'test',
'display_name': 'test', # defaults to dest 'display_name': 'test', # defaults to dest
'exclusive': False, 'exclusive': False,