Repository: trustyou/comfy

Issue:
Support changing and deleting items
Support assignment to existing options like so:

    config.section.opt = 2

The type of the assigned value should correspond to the option:

* `Option` takes strings, and raises ValueError otherwise
* `IntOption` takes integers …
* Etc.

The changed value should be reflected in the underlying `ConfigParser`, but not in the config files on disk.

Likewise, this should work:

    del config.section.opt

… and delete the value from the `ConfigParser`'s section.

Discussion:

Patch:
diff --git a/comfy/option.py b/comfy/option.py
index 1aacfd9..47cb692 100644
--- a/comfy/option.py
+++ b/comfy/option.py
@@ -1,41 +1,62 @@
-from typing import List, Optional
+from typing import List, Optional, Text, Any
 
 from comfy.schema import BaseOption, Section
 
 
 class Option(BaseOption):
 
-    def __get__(self, section, type=None):
-        # type: (Section, Optional[type]) -> str
-        config_parser = section.config_parser
-        return config_parser.get(section.name, self.name)
+    def unserialize(self, value):
+        # type: (Text) -> Text
+        return value
+
+    def serialize(self, new_value):
+        # type: (Text) -> Text
+        if not isinstance(new_value, str):
+            raise ValueError("New value should be a string.")
+        return str(new_value)
 
 
 class IntOption(BaseOption):
 
-    def __get__(self, section, type=None):
-        # type: (Section, Optional[type]) -> int
-        config_parser = section.config_parser
-        return config_parser.getint(section.name, self.name)
+    def unserialize(self, raw_value):
+        # type: (Text) -> int
+        return int(raw_value)
+
+    def serialize(self, new_value):
+        # type: (int) -> Text
+        if not isinstance(new_value, int):
+            raise ValueError("New value should be an int.")
+
+        return str(new_value)
 
 
 class BoolOption(BaseOption):
 
     truth_values = {"1", "yes", "true"}
 
-    def __get__(self, section, type=None):
-        # type: (Section, Optional[type]) -> bool
-        config_parser = section.config_parser
-        value = config_parser.get(section.name, self.name)
-        return value.lower() in self.truth_values
+    def unserialize(self, raw_value):
+        # type: (Text) -> bool
+        return raw_value.lower() in self.truth_values
+
+    def serialize(self, new_value):
+        # type: (bool) -> Text
+        if not isinstance(new_value, bool):
+            raise ValueError("New value should be a boolean.")
+
+        return str(new_value).lower()
 
 
 class ListOption(BaseOption):
 
-    def __get__(self, section, type=None):
-        # type: (Section, Optional[type]) -> List[str]
-        config_parser = section.config_parser
-        values = config_parser.get(section.name, self.name)
-        if values.strip() == "":
+    def unserialize(self, raw_value):
+        # type: (Text) -> List[Any]
+        if raw_value.strip() == "":
             return []
-        return [value.strip() for value in values.split(",")]
+        return [value.strip() for value in raw_value.split(",")]
+
+    def serialize(self, new_value):
+        # type: (List[Any]) -> Text
+        if not isinstance(new_value, list):
+            raise ValueError("New value should be a list.")
+
+        return ','.join(str(val) for val in new_value)
diff --git a/comfy/schema.py b/comfy/schema.py
index ff2d523..d7ffe3d 100644
--- a/comfy/schema.py
+++ b/comfy/schema.py
@@ -5,7 +5,7 @@ except ImportError:
     from ConfigParser import ConfigParser, NoOptionError
 
 from inspect import getmembers, isclass
-from typing import Any, Optional, Type
+from typing import Any, Optional, Type, Text
 
 from comfy.util import camel_case_to_lower
 
@@ -77,24 +77,49 @@ class Section:
 
 class BaseOption:
     """
-    Base class for options. Subclass this and override __get__ to define new option types.
+    Base class for options.
+
+    Subclass this and override unserialize() and serialize() to define new option types.
     """
 
     def __init__(self):
         self.name = None  # type: str
 
-    def __get__(self, obj, type=None):
-        # type: (Section, Optional[type]) -> Any
+    def unserialize(self, value):
+        # type: (Text) -> Any
         # Override in sub classes.
         raise NotImplementedError()
 
-    def __set__(self, obj, value):
-        # Assignment currently unsupported.
-        raise AttributeError()
+    def serialize(self, new_value):
+        # type: (Any) -> Text
+        """Checks if the value corresponds to the type of option.
+
+        Returns a string, which will be stored in the config_parser instance.
+
+        Raises ValueError if new_value does not meet expectations.
+        """
+        raise NotImplementedError()
+
+    def __get__(self, section, type=None):
+        # type: (Section, Optional[type]) -> Any
+        config_parser = section.config_parser
+
+        raw_value = config_parser.get(section.name, self.name)
+
+        return self.unserialize(raw_value)
+
+    def __set__(self, section, new_value):
+        # type: (Section, Any) -> None
+        config_parser = section.config_parser
+
+        raw_value = self.serialize(new_value)
+
+        config_parser.set(section.name, self.name, raw_value)
+
+    def __delete__(self, section):
+        config_parser = section.config_parser
 
-    def __delete__(self, obj):
-        # Deletion currently unsupported.
-        raise AttributeError()
+        config_parser.remove_option(section.name, self.name)
 
     def set_name(self, name):
         # type: (str) -> None


Test patch:
diff --git a/tests/test_schema.py b/tests/test_schema.py
index 5c1ab0d..5a0a075 100644
--- a/tests/test_schema.py
+++ b/tests/test_schema.py
@@ -42,11 +42,15 @@ def test_smoke(my_config):
     assert my_config.tmp_directories.vid_tmp == "/tmp/vid"
     assert my_config.tmp_directories.max_files == 1024
 
-    # Assignment and deletion are currently unsupported!
-    with pytest.raises(AttributeError):
-        my_config.tmp_directories.vid_tmp = "/tmp/videos"
-    with pytest.raises(AttributeError):
-        del my_config.tmp_directories.vid_tmp
+    new_val = "/tmp/videos"
+    my_config.tmp_directories.vid_tmp = new_val
+    assert my_config.tmp_directories.vid_tmp == new_val
+    assert my_config.config_parser.get("tmp_directories", "vid_tmp") == new_val
+
+    del my_config.tmp_directories.vid_tmp
+    with pytest.raises(NoOptionError):
+        assert my_config.tmp_directories.vid_tmp == new_val
+
 
 def _test_error(config, Error):
     # type: (str, Exception) -> None