Improve exception management and error message when doing disambiguation

[dojeda]

In the current example code and the testing code for the deserialization disambiguation function, there are a couple of problems:

def shape_schema_deserialization_disambiguation(object_dict, parent_object_dict):
    if object_dict.get("base"):
        return TriangleSchema()
    elif object_dict.get("length"):
        return RectangleSchema()

    raise TypeError("Could not detect type. "
                    "Did not have a base or a length. "
                    "Are you sure this is a shape?")

First, the check on the existence of the base key would fail if one uses an object that evaluates to false, giving a TypeError instead of whatever ValidationError the TriangleSchema would give:

>>> TestPolyField.ContrivedShapeClassSchema().load({'main': {'base': '', 'height': 1, 'color': 'red'}})
UnmarshalResult(data={}, errors={'main': ["Unable to use schema. Ensure there is a deserialization_schema_selector and that it returns a schema when the function is passed in {'height': 1, 'base': '', 'color': 'red'}. This is the class I got. Make sure it is a schema: None"]})

This can be easily fixed with an approach like the one represented in this diff:

diff --git a/tests/shapes.py b/tests/shapes.py
index 06ff8a1..5a97b02 100644
--- a/tests/shapes.py
+++ b/tests/shapes.py
@@ -85,9 +85,9 @@ def shape_property_schema_serialization_disambiguation(base_object, obj):
 
 
 def shape_schema_deserialization_disambiguation(object_dict, _):
-    if object_dict.get("base"):
+    if 'base' in object_dict:
         return TriangleSchema()
-    elif object_dict.get("length"):
+    elif 'length' in object_dict:
         return RectangleSchema()
 
     raise TypeError("Could not detect type. "

Secondly, if we would arrive at the part where the TypeError is raised, this exception is captured and lost because PolyField._deserialize ignores whatever Exception is thrown and throws a new ValidationError with a specific message:

            try:
                schema = self.deserialization_schema_selector(v, data)
                assert hasattr(schema, 'load')
            except Exception:
                schema_message = None
                if schema:
                    schema_message = str(type(schema))

                raise ValidationError(
                    "Unable to use schema. Ensure there is a deserialization_schema_selector"
                    " and that it returns a schema when the function is passed in {value_passed}."
                    " This is the class I got. Make sure it is a schema: {class_type}".format(
                        value_passed=v,
                        class_type=schema_message
                    )
                )

If one wanted to deserialize an object that cannot be disambiguated, it fills the error result with the following message that is difficult to understand:

>>> TestPolyField.ContrivedShapeClassSchema().load({'main': {}})
UnmarshalResult(data={}, errors={'main': ["Unable to use schema. Ensure there is a deserialization_schema_selector and that it returns a schema when the function is passed in {'height': 1, 'color': 'red'}. This is the class I got. Make sure it is a schema: None"]})

It would be very useful if the PolyField._deserialize would propagate the TypeError (I would personally prefer to propagate ValidationErrors for that matter) so that the user code can set its own validation error message. A simple modification such as:

diff --git a/marshmallow_polyfield/polyfield.py b/marshmallow_polyfield/polyfield.py
index 6b526c7..6824665 100644
--- a/marshmallow_polyfield/polyfield.py
+++ b/marshmallow_polyfield/polyfield.py
@@ -40,6 +40,14 @@ class PolyField(Field):
             try:
                 schema = self.deserialization_schema_selector(v, data)
                 assert hasattr(schema, 'load')
+            except TypeError as te:
+                # Convert to a ValidationError, but consider some alternatives:
+                # ValidationError(str(TypeError)) from te
+                # six.raise_from(ValidationError(str(TypeError)), te)
+                raise ValidationError(str(te))
+            except ValidationError:
+                # Propagate the ValidationError
+                raise
             except Exception:
                 schema_message = None
                 if schema:

Gives a more explicit error message, due to a better handling of the exceptions:

>>> TestPolyField.ContrivedShapeClassSchema().load({'main': {}})
UnmarshalResult(data={}, errors={'main': ['Could not detect type. Did not have a base or a length. Are you sure this is a shape?']})

Do you think this would be a good improvement to this library? What do you think about propagating ValidationErrors ? I am happy to fork and PR, where the documentation is updated as well.

[Bachmann1234]

Id be game. Open a PR and ill take a look and likely merge.

[pdamodara]

I ran across a similar issue today. However, rather than providing extra logic for TypeError specifically, I think it would be useful to provide similar functionality/handling for any uncaught Exception.

The simplest fix would be to catch Exception as a variable, and add that on to the ValidationError.
Also +1 for using six.raise_from.

I'll set up a PR with this change, we can discuss it there.

Update: Created this PR - #22