From f4dfcc3c18654c8b8f991aa8b78ad0d00bad0ca9 Mon Sep 17 00:00:00 2001
From: "jon.crall" <jon.crall@kitware.com>
Date: Fri, 3 Jan 2025 13:41:57 -0500
Subject: [PATCH] refactor: Hook up allow_split and add warning of future
 backwards incompatibility

---
 CHANGELOG.md              |  2 ++
 scriptconfig/smartcast.py | 45 +++++++++++++++++++++++++++++++++------
 scriptconfig/value.py     |  4 +++-
 3 files changed, 43 insertions(+), 8 deletions(-)

diff --git a/CHANGELOG.md b/CHANGELOG.md
index 1a9639a..21590cc 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -12,6 +12,8 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
   interoperability with argparse. We may remove the `value` keyword in the
   future in favor of this.
 
+* The `smartcast` `allow_split` now works, and defaults to "auto", which will prepare us for a backwards incompatible change to remove the auto string split behavior.
+
 
 ### Added
 * Add experimental new method `DataConfig.cls_from_argparse` which dynamically
diff --git a/scriptconfig/smartcast.py b/scriptconfig/smartcast.py
index 821f510..733ec32 100644
--- a/scriptconfig/smartcast.py
+++ b/scriptconfig/smartcast.py
@@ -3,7 +3,7 @@ __all__ = ['smartcast']
 NoneType = type(None)
 
 
-def smartcast(item, astype=None, strict=False, allow_split=False):
+def smartcast(item, astype=None, strict=False, allow_split='auto'):
     r"""
     Converts a string into a standard python type.
 
@@ -15,7 +15,7 @@ def smartcast(item, astype=None, strict=False, allow_split=False):
     list, tuple.
 
     Args:
-        item (str | object):
+        item (str | Any):
             represents some data of another type.
 
         astype (type | None):
@@ -29,10 +29,11 @@ def smartcast(item, astype=None, strict=False, allow_split=False):
 
         allow_split (bool):
             if True will interpret strings with commas as sequences.
-            Defaults to True.
+            Defaults to "auto", which pre 1.0 will default to True and warn the
+            user. In version 1.0 we will change the default to False.
 
     Returns:
-        object: some item
+        Any: some item
 
     Raises:
         TypeError: if we cannot determine the type
@@ -92,12 +93,42 @@ def smartcast(item, astype=None, strict=False, allow_split=False):
 
     if isinstance(item, str):
         if astype is None:
-            type_list = [int, float, complex, bool, NoneType]
+            candidate_type_list = [int, float, complex, bool, NoneType]
             if ',' in item:
-                type_list += [list, tuple, set]
+                # NOTE: THIS TRIES TO BE TOO CLEVER AND FAILS. We need to
+                # depreate this behavior where it will automagically split
+                # commas. We need to simplify the behavior and have the user
+                # explicitly enable similar behavior.
+
+                # The auto int / float / bool parts are fine. The auto list
+                # part is what is causing the problem. Perhaps we should just
+                # use YAML.
+
+                # Plan:
+                # 1. Add a allow_split flag that defaults to 'auto' and if this case
+                # is hit.
+                # 2. If this case is hit and the allow_split flag is auto, warn the
+                # user that the behavior will change in the future.
+                # 3. For now have auto default to True.
+                # 4. In the future change the default to False.
+
+                if allow_split == 'auto':
+                    import warnings
+                    warnings.warn(
+                        'smartcast has been given a string with commas and the '
+                        'allow_split="auto". Currently this will default to True and split the string into a list. '
+                        'After version 1.0 the default will change to False and strings will not be split into lists by default '
+                        'To disable this warning explicitly set allow_split=True to keep the string splitting behavior or allow_split=False '
+                        'to disable it and use the new default behavior. '
+                        'If using this in a Value object, you can prevent future incompatibility by '
+                        'setting type=str and handling casting in the __post_init__ method of the DataConfig'
+                    )
+                    allow_split = True
+                if allow_split:
+                    candidate_type_list += [list, tuple, set]
 
             # Try each candidate in the type list until something works
-            for astype in type_list:
+            for astype in candidate_type_list:
                 try:
                     return _as_smart_type(item, astype)
                 except (TypeError, ValueError):
diff --git a/scriptconfig/value.py b/scriptconfig/value.py
index 851c58a..a7a551b 100644
--- a/scriptconfig/value.py
+++ b/scriptconfig/value.py
@@ -130,7 +130,9 @@ class Value(ub.NiceRepr):
 
     def cast(self, value):
         if isinstance(value, str):
-            value = smartcast_mod.smartcast(value, self.type)
+            # FIXME: We want to move away from allow_split=True
+            value = smartcast_mod.smartcast(value, self.type,
+                                            allow_split='auto')
         return value
 
     def copy(self):
-- 
GitLab