Repository: dalemyers/deserialize (language: python)

Issue:
Attribute parser gets redefined by another subclass of common ancestor
When a decorator defines a parser for an attribute in a class then that definition is also applied to other subclasses of the same base class.


```python
import decimal
from typing import Any

import attr
import deserialize


def _money_amount(value: Any):
    return decimal.Decimal(value).quantize(decimal.Decimal("0.01"), decimal.ROUND_HALF_UP) if value else None


@attr.s(auto_attribs=True)
@deserialize.parser("a", _money_amount)
class Base:
    a: decimal.Decimal


@attr.s(auto_attribs=True)
class Foo(Base):
    b: str


@attr.s(auto_attribs=True)
@deserialize.parser("b", _money_amount)
class Bar(Base):
    b: decimal.Decimal


def test_deserialize_base():
    deserialize.deserialize(Base, {"a": 1.23})


def test_deserialize_foo():
    deserialize.deserialize(Foo, {"a": 1.23, "b": "b"})


def test_deserialize_bar():
    deserialize.deserialize(Bar, {"a": 1.23, "b": 1.23})
```


test_deserlalize.py::test_deserialize_base PASSED
test_deserlalize.py::test_deserialize_bar PASSED 
 

tests/test_deserlalize.py:32 (test_deserialize_foo)
def test_deserialize_foo():
>       deserialize.deserialize(Foo, {"a": 1.23, "b": "b"})

test_deserlalize.py:34: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
../../../opt/anaconda3/envs/fractal-python/lib/python3.9/site-packages/deserialize/__init__.py:93: in deserialize
    return _deserialize(
../../../opt/anaconda3/envs/fractal-python/lib/python3.9/site-packages/deserialize/__init__.py:175: in _deserialize
    _deserialize_dict(
../../../opt/anaconda3/envs/fractal-python/lib/python3.9/site-packages/deserialize/__init__.py:370: in _deserialize_dict
    property_value = parser_function(value)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

value = 'b'

    def _money_amount(value: Any):
>       return decimal.Decimal(value).quantize(decimal.Decimal("0.01"), decimal.ROUND_HALF_UP) if value else None
E       decimal.InvalidOperation: [<class 'decimal.ConversionSyntax'>]

test_deserlalize.py:9: InvalidOperation




Discussion / hints:
Thanks for the report and solid repro steps! I'll try and take a look today. 

Fix patch:
diff --git a/deserialize/decorators/base.py b/deserialize/decorators/base.py
new file mode 100644
index 0000000..4d4bdaa
--- /dev/null
+++ b/deserialize/decorators/base.py
@@ -0,0 +1,49 @@
+"""Decorators used for adding functionality to the library."""
+
+import inspect
+
+
+def _generate_key(class_reference):
+    """Generate a key for lookups."""
+    return str(class_reference)
+
+
+def set_property(class_reference, property_name, key_name, property_value):
+    """A apply a property to a class."""
+
+    if not hasattr(class_reference, property_name):
+        setattr(class_reference, property_name, {})
+
+    class_key = _generate_key(class_reference)
+
+    subclass_map = getattr(class_reference, property_name, {}).get(class_key)
+
+    if subclass_map is None:
+        getattr(class_reference, property_name)[class_key] = {}
+
+    getattr(class_reference, property_name)[class_key][key_name] = property_value
+
+
+def get_property(class_reference, property_name, key_name, sentinel_value=None):
+    """Get the property for the given class, property and key name."""
+
+    if not hasattr(class_reference, property_name):
+        return sentinel_value
+
+    class_key = class_key = _generate_key(class_reference)
+    subclass_map = getattr(class_reference, property_name, {}).get(class_key)
+
+    if subclass_map:
+        value = subclass_map.get(key_name, sentinel_value)
+        if value != sentinel_value:
+            return value
+
+    for superclass in inspect.getmro(class_reference):
+        if superclass == class_reference:
+            continue
+
+        value = get_property(superclass, property_name, key_name, sentinel_value)
+        if value != sentinel_value:
+            return value
+
+    return sentinel_value
diff --git a/deserialize/decorators/constructed.py b/deserialize/decorators/constructed.py
index 99c1a35..04c0f5e 100644
--- a/deserialize/decorators/constructed.py
+++ b/deserialize/decorators/constructed.py
@@ -1,12 +1,14 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
 
 def constructed(function):
     """A decorator function for calling when a class is constructed."""
 
     def store_constructed(class_reference):
         """Store the key map."""
-        setattr(class_reference, "__deserialize_constructed__", function)
+        base.set_property(class_reference, "__deserialize_constructed__", "constructed", function)
         return class_reference
 
     return store_constructed
@@ -15,7 +17,6 @@ def store_constructed(class_reference):
 def _call_constructed(class_reference, instance):
     """Check if a property is allowed to be unhandled."""
 
-    if not hasattr(class_reference, "__deserialize_constructed__"):
-        return
-
-    class_reference.__deserialize_constructed__(instance)
+    constructor = base.get_property(class_reference, "__deserialize_constructed__", "constructed")
+    if constructor:
+        constructor(instance)
diff --git a/deserialize/decorators/default.py b/deserialize/decorators/default.py
index 6873156..ea8d0d4 100644
--- a/deserialize/decorators/default.py
+++ b/deserialize/decorators/default.py
@@ -1,6 +1,7 @@
 """Decorators used for adding functionality to the library."""
 
 from deserialize.exceptions import NoDefaultSpecifiedException
+from deserialize.decorators import base
 
 
 def default(key_name, default_value):
@@ -8,12 +9,7 @@ def default(key_name, default_value):
 
     def store_defaults_map(class_reference):
         """Store the defaults map."""
-
-        if not hasattr(class_reference, "__deserialize_defaults_map__"):
-            setattr(class_reference, "__deserialize_defaults_map__", {})
-
-        class_reference.__deserialize_defaults_map__[key_name] = default_value
-
+        base.set_property(class_reference, "__deserialize_defaults_map__", key_name, default_value)
         return class_reference
 
     return store_defaults_map
@@ -25,10 +21,13 @@ def _has_default(class_reference, key_name):
     :returns: True if this key has a default, False otherwise.
     """
 
-    if not hasattr(class_reference, "__deserialize_defaults_map__"):
-        return False
+    sentinel = object()
 
-    return key_name in class_reference.__deserialize_defaults_map__
+    default_value = base.get_property(
+        class_reference, "__deserialize_defaults_map__", key_name, sentinel
+    )
+
+    return default_value != sentinel
 
 
 def _get_default(class_reference, key_name):
@@ -37,10 +36,13 @@ def _get_default(class_reference, key_name):
     :raises NoDefaultSpecifiedException: If a default hasn't been specified
     """
 
-    if not hasattr(class_reference, "__deserialize_defaults_map__"):
-        raise NoDefaultSpecifiedException()
+    sentinel = object()
+
+    default_value = base.get_property(
+        class_reference, "__deserialize_defaults_map__", key_name, sentinel
+    )
 
-    if key_name in class_reference.__deserialize_defaults_map__:
-        return class_reference.__deserialize_defaults_map__[key_name]
+    if default_value != sentinel:
+        return default_value
 
     raise NoDefaultSpecifiedException()
diff --git a/deserialize/decorators/downcasting.py b/deserialize/decorators/downcasting.py
index e725856..b5b1754 100644
--- a/deserialize/decorators/downcasting.py
+++ b/deserialize/decorators/downcasting.py
@@ -1,12 +1,14 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
 
 def downcast_field(property_name):
     """A decorator function for handling downcasting."""
 
     def store_downcast_field_value(class_reference):
         """Store the key map."""
-        setattr(class_reference, "__deserialize_downcast_field__", property_name)
+        base.set_property(class_reference, "__deserialize_downcast_field__", "value", property_name)
         return class_reference
 
     return store_downcast_field_value
@@ -14,7 +16,7 @@ def store_downcast_field_value(class_reference):
 
 def _get_downcast_field(class_reference):
     """Get the downcast field name if set, None otherwise."""
-    return getattr(class_reference, "__deserialize_downcast_field__", None)
+    return base.get_property(class_reference, "__deserialize_downcast_field__", "value")
 
 
 def downcast_identifier(super_class, identifier):
@@ -22,11 +24,7 @@ def downcast_identifier(super_class, identifier):
 
     def store_key_map(class_reference):
         """Store the downcast map."""
-        if not hasattr(super_class, "__deserialize_downcast_map__"):
-            setattr(super_class, "__deserialize_downcast_map__", {})
-
-        super_class.__deserialize_downcast_map__[identifier] = class_reference
-
+        base.set_property(super_class, "__deserialize_downcast_map__", identifier, class_reference)
         return class_reference
 
     return store_key_map
@@ -34,11 +32,7 @@ def store_key_map(class_reference):
 
 def _get_downcast_class(super_class, identifier):
     """Get the downcast identifier for the given class and super class, returning None if not set"""
-
-    if not hasattr(super_class, "__deserialize_downcast_map__"):
-        return None
-
-    return super_class.__deserialize_downcast_map__.get(identifier)
+    return base.get_property(super_class, "__deserialize_downcast_map__", identifier)
 
 
 def allow_downcast_fallback():
@@ -46,7 +40,7 @@ def allow_downcast_fallback():
 
     def store(class_reference):
         """Store the allowance flag."""
-        setattr(class_reference, "__deserialize_downcast_allow_fallback__", True)
+        base.set_property(class_reference, "__deserialize_downcast_allow_fallback__", "value", True)
         return class_reference
 
     return store
@@ -54,4 +48,4 @@ def store(class_reference):
 
 def _allows_downcast_fallback(super_class):
     """Get the whether downcast can fallback to a dict or not"""
-    return getattr(super_class, "__deserialize_downcast_allow_fallback__", False)
+    return base.get_property(super_class, "__deserialize_downcast_allow_fallback__", "value", False)
diff --git a/deserialize/decorators/ignore.py b/deserialize/decorators/ignore.py
index b3dfe79..82f0700 100644
--- a/deserialize/decorators/ignore.py
+++ b/deserialize/decorators/ignore.py
@@ -1,16 +1,14 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
 
 def ignore(property_name):
     """A decorator function for marking keys as those which should be ignored."""
 
     def store_key_map(class_reference):
         """Store the key map."""
-        if not hasattr(class_reference, "__deserialize_ignore_map__"):
-            setattr(class_reference, "__deserialize_ignore_map__", {})
-
-        class_reference.__deserialize_ignore_map__[property_name] = True
-
+        base.set_property(class_reference, "__deserialize_ignore_map__", property_name, True)
         return class_reference
 
     return store_key_map
@@ -18,8 +16,4 @@ def store_key_map(class_reference):
 
 def _should_ignore(class_reference, property_name):
     """Check if a property should be ignored."""
-
-    if not hasattr(class_reference, "__deserialize_ignore_map__"):
-        return False
-
-    return class_reference.__deserialize_ignore_map__.get(property_name, False)
+    return base.get_property(class_reference, "__deserialize_ignore_map__", property_name, False)
diff --git a/deserialize/decorators/key.py b/deserialize/decorators/key.py
index 942798b..3e3f0ed 100644
--- a/deserialize/decorators/key.py
+++ b/deserialize/decorators/key.py
@@ -1,16 +1,15 @@
 """Decorators used for adding functionality to the library."""
 
 
+from deserialize.decorators import base
+
+
 def key(property_name, key_name):
     """A decorator function for mapping key names to properties."""
 
     def store_key_map(class_reference):
         """Store the key map."""
-        if not hasattr(class_reference, "__deserialize_key_map__"):
-            setattr(class_reference, "__deserialize_key_map__", {})
-
-        class_reference.__deserialize_key_map__[property_name] = key_name
-
+        base.set_property(class_reference, "__deserialize_key_map__", property_name, key_name)
         return class_reference
 
     return store_key_map
@@ -19,7 +18,6 @@ def store_key_map(class_reference):
 def _get_key(class_reference, property_name):
     """Get the key for the given class and property name."""
 
-    if not hasattr(class_reference, "__deserialize_key_map__"):
-        return property_name
-
-    return class_reference.__deserialize_key_map__.get(property_name, property_name)
+    return base.get_property(
+        class_reference, "__deserialize_key_map__", property_name, property_name
+    )
diff --git a/deserialize/decorators/parser.py b/deserialize/decorators/parser.py
index 12a4623..36c4e39 100644
--- a/deserialize/decorators/parser.py
+++ b/deserialize/decorators/parser.py
@@ -1,17 +1,19 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
+
+def _identity_parser(value):
+    """This parser does nothing. It's simply used as the default."""
+    return value
+
 
 def parser(key_name, parser_function):
     """A decorator function for mapping parsers to key names."""
 
     def store_parser_map(class_reference):
         """Store the parser map."""
-
-        if not hasattr(class_reference, "__deserialize_parser_map__"):
-            setattr(class_reference, "__deserialize_parser_map__", {})
-
-        class_reference.__deserialize_parser_map__[key_name] = parser_function
-
+        base.set_property(class_reference, "__deserialize_parser_map__", key_name, parser_function)
         return class_reference
 
     return store_parser_map
@@ -19,12 +21,6 @@ def store_parser_map(class_reference):
 
 def _get_parser(class_reference, key_name):
     """Get the parser for the given class and key name."""
-
-    def identity_parser(value):
-        """This parser does nothing. It's simply used as the default."""
-        return value
-
-    if not hasattr(class_reference, "__deserialize_parser_map__"):
-        return identity_parser
-
-    return class_reference.__deserialize_parser_map__.get(key_name, identity_parser)
+    return base.get_property(
+        class_reference, "__deserialize_parser_map__", key_name, _identity_parser
+    )
diff --git a/deserialize/decorators/snake.py b/deserialize/decorators/snake.py
index 1ec9361..50b7f29 100644
--- a/deserialize/decorators/snake.py
+++ b/deserialize/decorators/snake.py
@@ -1,17 +1,19 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
 
 def auto_snake():
     """A decorator function for marking classes as those which should be auto-snaked."""
 
     def store(class_reference):
         """Store the allowance flag."""
-        setattr(class_reference, "__deserialize_auto_snake__", True)
+        base.set_property(class_reference, "__deserialize_auto_snake__", "value", True)
         return class_reference
 
     return store
 
 
-def _uses_auto_snake(super_class):
+def _uses_auto_snake(class_reference):
     """Get the whether auto-snake is in use or not"""
-    return getattr(super_class, "__deserialize_auto_snake__", False)
+    return base.get_property(class_reference, "__deserialize_auto_snake__", "value", False)
diff --git a/deserialize/decorators/unhandled.py b/deserialize/decorators/unhandled.py
index 2e21d2c..58fee69 100644
--- a/deserialize/decorators/unhandled.py
+++ b/deserialize/decorators/unhandled.py
@@ -1,16 +1,14 @@
 """Decorators used for adding functionality to the library."""
 
+from deserialize.decorators import base
+
 
 def allow_unhandled(key_name):
     """A decorator function for marking keys as allowed to be unhandled."""
 
     def store_allow_unhandled_map(class_reference):
         """Store the key map."""
-        if not hasattr(class_reference, "__deserialize_allow_unhandled_map__"):
-            setattr(class_reference, "__deserialize_allow_unhandled_map__", {})
-
-        class_reference.__deserialize_allow_unhandled_map__[key_name] = True
-
+        base.set_property(class_reference, "__deserialize_allow_unhandled_map__", key_name, True)
         return class_reference
 
     return store_allow_unhandled_map
@@ -19,7 +17,6 @@ def store_allow_unhandled_map(class_reference):
 def _should_allow_unhandled(class_reference, key_name):
     """Check if a property is allowed to be unhandled."""
 
-    if not hasattr(class_reference, "__deserialize_allow_unhandled_map__"):
-        return False
-
-    return class_reference.__deserialize_allow_unhandled_map__.get(key_name, False)
+    return base.get_property(
+        class_reference, "__deserialize_allow_unhandled_map__", key_name, False
+    )


Test patch:
diff --git a/tests/test_enums.py b/tests/test_enums.py
index d943214..07674dd 100755
--- a/tests/test_enums.py
+++ b/tests/test_enums.py
@@ -93,4 +93,3 @@ class OrderTest(enum.Enum):
 
         assert len(result) == len(data) == len(expected_result)
         assert result == expected_result
-
diff --git a/tests/test_parser_subclassing.py b/tests/test_parser_subclassing.py
new file mode 100755
index 0000000..493b5f8
--- /dev/null
+++ b/tests/test_parser_subclassing.py
@@ -0,0 +1,50 @@
+"""Test deserializing."""
+
+import decimal
+import os
+import sys
+from typing import Any
+
+sys.path.insert(0, os.path.abspath(os.path.join(os.path.dirname(__file__), "..")))
+# pylint: disable=wrong-import-position
+import deserialize
+
+# pylint: enable=wrong-import-position
+
+
+def _money_amount(value: Any):
+    return decimal.Decimal(value) if value else None
+
+
+@deserialize.parser("a", _money_amount)
+class Base:
+    a: decimal.Decimal
+
+
+class Foo(Base):
+    b: str
+
+
+@deserialize.parser("b", _money_amount)
+class Bar(Base):
+    b: decimal.Decimal
+
+
+class Baz(Bar):
+    pass
+
+
+def test_deserialize_base():
+    deserialize.deserialize(Base, {"a": 1.23})
+
+
+def test_deserialize_foo():
+    deserialize.deserialize(Foo, {"a": 1.23, "b": "b"})
+
+
+def test_deserialize_bar():
+    deserialize.deserialize(Bar, {"a": 1.23, "b": 1.23})
+
+
+def test_deserialize_baz():
+    deserialize.deserialize(Bar, {"a": 1.23, "b": 1.23})