From 4c385667e84db60fbc5e2effe5fbb63fed1b6c24 Mon Sep 17 00:00:00 2001 From: Timmy Welch Date: Sat, 16 Dec 2023 16:51:00 -0800 Subject: [PATCH] Fix settings being overwritten when using the same dest attribute --- settngs/__init__.py | 25 ++++++++++++++++--------- testing/settngs.py | 15 +++++++++++---- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/settngs/__init__.py b/settngs/__init__.py index f5a11cb..b45f70a 100644 --- a/settngs/__init__.py +++ b/settngs/__init__.py @@ -107,6 +107,9 @@ class Setting: 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: *names: Passed directly to argparse @@ -119,7 +122,8 @@ class Setting: required: Passed directly to argparse help: Passed directly to argparse 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. Defaults to `dest`. cmdline: If this setting can be set via the commandline @@ -133,7 +137,7 @@ class Setting: raise ValueError('names must be specified') # 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 - 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 # 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.metavar = metavar self.dest = dest + self.setting_name = setting_name self.cmdline = cmdline self.file = file self.argparse_args = args @@ -228,28 +233,30 @@ class Setting: return None return 'Any' - def get_dest(self, prefix: str, names: Sequence[str], dest: str | None) -> tuple[str, str, bool]: - dest_name = None + def get_dest(self, prefix: str, names: Sequence[str], dest: str | None) -> tuple[str, str, str, bool]: + setting_name = None flag = False prefix = sanitize_name(prefix) for n in names: if n.startswith('--'): flag = True - dest_name = sanitize_name(n) + setting_name = sanitize_name(n) break if n.startswith('-'): flag = True - if dest_name is None: - dest_name = names[0] + if setting_name is None: + setting_name = names[0] if dest: dest_name = dest + else: + dest_name = setting_name if not dest_name.isidentifier(): raise Exception(f'Cannot use {dest_name} in a namespace') 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]: 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""" 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: """ diff --git a/testing/settngs.py b/testing/settngs.py index f113742..620888b 100644 --- a/testing/settngs.py +++ b/testing/settngs.py @@ -77,6 +77,7 @@ success = [ 'cmdline': True, 'const': 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 'display_name': 'test_setting', # defaults to dest 'exclusive': False, @@ -84,7 +85,7 @@ success = [ 'flag': True, 'group': 'tst', '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 'nargs': None, 'required': None, @@ -118,6 +119,7 @@ success = [ 'cmdline': True, 'const': 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 'display_name': 'testing', # defaults to dest 'exclusive': False, @@ -125,7 +127,7 @@ success = [ 'flag': True, 'group': 'tst', '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 'nargs': None, 'required': None, @@ -158,6 +160,7 @@ success = [ 'cmdline': True, 'const': 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 'display_name': 'test', # defaults to dest 'exclusive': False, @@ -165,7 +168,7 @@ success = [ 'flag': True, 'group': 'tst', '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 'nargs': None, 'required': None, @@ -199,6 +202,7 @@ success = [ 'cmdline': True, 'const': 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 'display_name': 'test', # defaults to dest 'exclusive': False, @@ -206,7 +210,7 @@ success = [ 'flag': True, 'group': 'tst', '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 'nargs': None, 'required': None, @@ -239,6 +243,7 @@ success = [ 'cmdline': True, 'const': None, 'default': None, + 'setting_name': 'test', 'dest': 'test', 'display_name': 'test', # defaults to dest 'exclusive': False, @@ -279,6 +284,7 @@ success = [ 'cmdline': True, 'const': None, 'default': None, + 'setting_name': 'test', 'dest': 'test', 'display_name': 'test', # defaults to dest 'exclusive': False, @@ -317,6 +323,7 @@ success = [ 'cmdline': True, 'const': None, 'default': None, + 'setting_name': 'test', 'dest': 'test', 'display_name': 'test', # defaults to dest 'exclusive': False,