From 3f4271ce620202f44eb65130994f92238e59cecf Mon Sep 17 00:00:00 2001
From: joncrall <jon.crall@kitware.com>
Date: Fri, 3 Jan 2025 14:23:21 -0500
Subject: [PATCH] fix: issue with isflag and type specs. Add support for legacy
behavior
---
CHANGELOG.md | 4 +++
scriptconfig/argparse_ext.py | 10 +++++--
scriptconfig/value.py | 24 +++++++++++++++-
tests/test_smartcast_issues.py | 50 ++++++++++++++++++++++++++++++++++
4 files changed, 84 insertions(+), 4 deletions(-)
create mode 100644 tests/test_smartcast_issues.py
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 21590cc..a8f698f 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -6,6 +6,10 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
## Version 0.8.2 - Unreleased
+### Added
+
+* The "type" of a `Value` can now be set to "smartcast:legacy" for explicit old behavior or "smartcast:v1" for the new candidate behavior.
+
### Changed
* The `Value` class can now take `default` as a keyword argument for better
diff --git a/scriptconfig/argparse_ext.py b/scriptconfig/argparse_ext.py
index 5831a80..4370b20 100644
--- a/scriptconfig/argparse_ext.py
+++ b/scriptconfig/argparse_ext.py
@@ -96,7 +96,7 @@ class BooleanFlagOrKeyValAction(_Base):
>>> assert ns['flag'] == want
"""
def __init__(self, option_strings, dest, default=None, required=False,
- help=None):
+ help=None, type=None):
_option_strings = []
for option_string in option_strings:
@@ -109,11 +109,15 @@ class BooleanFlagOrKeyValAction(_Base):
actionkw = dict(
option_strings=_option_strings, dest=dest, default=default,
- type=None, choices=None, required=required, help=help,
+ type=type, choices=None, required=required, help=help,
metavar=None)
# Either the zero arg flag form or the 1 arg key/value form.
actionkw['nargs'] = '?'
+ # not sure if type is supported here. Hacking it in to fix
+ # interaction of smartcast and isflag
+ # self._hacked_in_type = type
+
# Hack because of the Store Base for configargparse support
argparse.Action.__init__(self, **actionkw)
# super().__init__(**actionkw)
@@ -148,7 +152,7 @@ class BooleanFlagOrKeyValAction(_Base):
else:
# Allow for non-boolean values (i.e. auto) to be passed
from scriptconfig import smartcast as smartcast_mod
- value = smartcast_mod.smartcast(values)
+ value = smartcast_mod.smartcast(values, action.type)
# value = smartcast_mod._smartcast_bool(values)
if not key_default:
value = not value
diff --git a/scriptconfig/value.py b/scriptconfig/value.py
index a7a551b..ebcfb53 100644
--- a/scriptconfig/value.py
+++ b/scriptconfig/value.py
@@ -267,6 +267,10 @@ class Path(Value):
"""
Note this is mean to be used only with scriptconfig.Config.
It does NOT represent a pathlib object.
+
+ NOTE:
+ Not well maintained or used, may be removed or refactored in the
+ future.
"""
def __init__(self, value=None, help=None, alias=None):
super(Path, self).__init__(value, str, help=help, alias=alias)
@@ -281,6 +285,10 @@ class PathList(Value):
"""
Can be specified as a list or as a globstr
+ NOTE:
+ Not well maintained or used, may be removed or refactored in the
+ future.
+
FIXME:
will fail if there are any commas in the path name
@@ -378,7 +386,7 @@ def _value_add_argument_to_parser(value, _value, self, parser, key, fuzzy_hyphen
if isflag:
# Can we support both flag and setitem methods of cli
# parsing?
- argkw.pop('type', None)
+ # argkw.pop('type', None)
argkw.pop('choices', None)
argkw.pop('action', None)
argkw.pop('nargs', None)
@@ -389,6 +397,20 @@ def _value_add_argument_to_parser(value, _value, self, parser, key, fuzzy_hyphen
else:
argkw['action'] = argparse_ext.BooleanFlagOrKeyValAction
+ if isinstance(argkw.get('type', None), str):
+ # Coerce the type into a callable. We may need to do this in other
+ # places if so, we should factor out common code.
+ if argkw['type'] == 'smartcast':
+ argkw['type'] = smartcast_mod.smartcast
+ elif argkw['type'] == 'smartcast:v1':
+ from functools import partial
+ argkw['type'] = partial(smartcast_mod.smartcast, allow_split=False)
+ elif argkw['type'] == 'smartcast:legacy':
+ from functools import partial
+ argkw['type'] = partial(smartcast_mod.smartcast, allow_split=True)
+ else:
+ raise KeyError(f'Unknown special type key: {argkw["type"]}')
+
try:
parent.add_argument(*option_strings, required=required, **argkw)
except Exception:
diff --git a/tests/test_smartcast_issues.py b/tests/test_smartcast_issues.py
new file mode 100644
index 0000000..40f2c7f
--- /dev/null
+++ b/tests/test_smartcast_issues.py
@@ -0,0 +1,50 @@
+
+
+def test_smartcast_interaction_with_isflag_type_is_respected():
+ """
+ In 0.8.1, when isflag=True, the type parameter was not respected.
+
+ CommandLine:
+ xdoctest -m tests/test_smartcast_issues.py test_smartcast_interaction_with_isflag_type_is_respected
+ """
+ import scriptconfig as scfg
+ import ubelt as ub
+
+ class MyConfig(scfg.DataConfig):
+ param1 = scfg.Value(1, type=str, isflag=True)
+ param2 = scfg.Value(2, type=str)
+ param3 = scfg.Value(3, type='smartcast:legacy', isflag=True)
+ param4 = scfg.Value(4, type='smartcast:legacy')
+
+ # Check that no splitting occurs when type=str and isflag=True
+ config1 = MyConfig.cli(argv='--param1=1,2,3 --param2=1,2,3 --param3=1,2,3 --param4=1,2,3')
+ types1 = ub.udict(config1).map_values(lambda x: type(x).__name__)
+ print(f'types = {ub.urepr(types1, nl=1)}')
+ assert types1 == {'param1': 'str', 'param2': 'str', 'param3': 'list', 'param4': 'list'}
+
+
+def test_smartcast_interaction_with_isflag_flagform_still_works():
+ """
+ Make sure that specifying a type of str does not impact the flagform of a CLI.
+
+ CommandLine:
+ xdoctest -m tests/test_smartcast_issues.py test_smartcast_interaction_with_isflag_flagform_still_works
+ """
+ # Check that no splitting occurs when type=str and isflag=True
+ import scriptconfig as scfg
+ import ubelt as ub
+
+ class MyConfig(scfg.DataConfig):
+ param1 = scfg.Value(0, type=str, isflag=True)
+ param3 = scfg.Value(0, type='smartcast:legacy', isflag=True)
+ config = dict(MyConfig.cli(argv=''))
+ print(f'config = {ub.urepr(config, nl=1)}')
+ assert config == {'param1': 0, 'param3': 0}
+
+ config = dict(MyConfig.cli(argv='--param1 --param3'))
+ print(f'config = {ub.urepr(config, nl=1)}')
+ assert config == {'param1': True, 'param3': True}
+
+ config = dict(MyConfig.cli(argv='--no-param1 --no-param3'))
+ print(f'config = {ub.urepr(config, nl=1)}')
+ assert config == {'param1': False, 'param3': False}
--
GitLab