diff --git a/CHANGELOG.md b/CHANGELOG.md index 21590cc303fb01ce3d98e19e186e8d7769ab3c46..a8f698f342c45c259c5cf992e07e9545d8c01c36 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 5831a80cf8e33dc663f65ec7608a45ab161353f5..4370b20fb8ffaf0c240ec8eb1887afcd0689e5e4 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 a7a551b66e0aef8eae084b3ffe53b6a556ac703c..ebcfb53232e24163a090f19b8ab44a5eecd1baec 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 0000000000000000000000000000000000000000..40f2c7f756b7b1d8f04116bcc3f89f4ab2a0f283 --- /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}