Repository: marshmallow-code/marshmallow
Pull request #1332 (2019-08-03 18:45:16)

Deprecate Field.fail in favor of Field.make_error
As pointed in https://github.com/marshmallow-code/marshmallow/pull/1301#issuecomment-511235599 , `fail` is less flexible than `make_error` and is no longer necessary.


Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index fd72c975..b7f6cdf2 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -9,6 +9,10 @@ Features:
 - Optimize ``List(Nested(...))`` (:issue:`779`).
 - Minor performance improvements and cleanup (:pr:`1328`).
 
+Deprecations/Removals:
+
+- ``Field.fail`` is deprecated. Use ``Field.make_error`` instead.
+
 Support:
 
 - Various docs improvements (:pr:`1329`).
diff --git a/docs/upgrading.rst b/docs/upgrading.rst
index 4e364f90..28a4e70d 100644
--- a/docs/upgrading.rst
+++ b/docs/upgrading.rst
@@ -997,6 +997,40 @@ In marshmallow 2, it was possible to have multiple fields with the same ``attrib
     # No error
 
 
+``Field.fail`` is deprecated in favor of ``Field.make_error``
+*************************************************************
+
+`Field.fail <marshmallow.fields.Field.fail>` is deprecated. 
+Use `Field.make_error <marshmallow.fields.Field.fail>`. This allows you to
+re-raise exceptions using ``raise ... from ...``.
+
+.. code-block:: python
+
+    from marshmallow import fields, ValidationError
+    from packaging import version
+
+    # 2.x
+    class Version(fields.Field):
+        default_error_messages = {"invalid": "Not a valid version."}
+
+        def _deserialize(self, value, *args, **kwargs):
+            try:
+                return version.Version(value)
+            except version.InvalidVersion:
+                self.fail("invalid")
+
+
+    # 3.x
+    class Version(fields.Field):
+        default_error_messages = {"invalid": "Not a valid version."}
+
+        def _deserialize(self, value, *args, **kwargs):
+            try:
+                return version.Version(value)
+            except version.InvalidVersion as error:
+                raise self.make_error("invalid") from error
+
+
 ``python-dateutil`` recommended dependency is removed
 *****************************************************
 
diff --git a/src/marshmallow/fields.py b/src/marshmallow/fields.py
index d25b3e77..13c18ee4 100644
--- a/src/marshmallow/fields.py
+++ b/src/marshmallow/fields.py
@@ -7,6 +7,7 @@ import numbers
 import uuid
 import decimal
 import math
+import warnings
 from collections.abc import Mapping as _Mapping
 
 from marshmallow import validate, utils, class_registry
@@ -74,7 +75,8 @@ class Field(FieldABC):
         input value is missing. May be a value or a callable.
     :param missing: Default deserialization value for the field if the field is not
         found in the input data. May be a value or a callable.
-    :param str data_key: The name of the output key in the output of `load` and `dump`.
+    :param str data_key: The name of the dict key in the external representation, i.e.
+        the input of `load` and the output of `dump`.
         If `None`, the key will match the name of the field.
     :param str attribute: The name of the attribute to get the value from when serializing.
         If `None`, assumes the attribute has the same name as the field.
@@ -236,7 +238,7 @@ class Field(FieldABC):
             try:
                 r = validator(value)
                 if not isinstance(validator, Validator) and r is False:
-                    self.fail("validator_failed")
+                    raise self.make_error("validator_failed")
             except ValidationError as err:
                 kwargs.update(err.kwargs)
                 if isinstance(err.messages, dict):
@@ -247,6 +249,9 @@ class Field(FieldABC):
             raise ValidationError(errors, **kwargs)
 
     def make_error(self, key: str, **kwargs) -> ValidationError:
+        """Helper method to make a `ValidationError` with an error message
+        from ``self.error_messages``.
+        """
         try:
             msg = self.error_messages[key]
         except KeyError as error:
@@ -257,10 +262,19 @@ class Field(FieldABC):
             msg = msg.format(**kwargs)
         return ValidationError(msg)
 
-    # Hat tip to django-rest-framework.
     def fail(self, key: str, **kwargs):
-        """A helper method that simply raises a `ValidationError`.
+        """Helper method that raises a `ValidationError` with an error message
+        from ``self.error_messages``.
+
+        .. deprecated:: 3.0.0
+            Use `make_error <marshmallow.fields.Field.make_error>` instead.
         """
+        warnings.warn(
+            '`Field.fail` is deprecated. Use `raise self.make_error("{}", ...)` instead.'.format(
+                key
+            ),
+            DeprecationWarning,
+        )
         raise self.make_error(key=key, **kwargs)
 
     def _validate_missing(self, value):
@@ -269,10 +283,10 @@ class Field(FieldABC):
         """
         if value is missing_:
             if hasattr(self, "required") and self.required:
-                self.fail("required")
+                raise self.make_error("required")
         if value is None:
             if hasattr(self, "allow_none") and self.allow_none is not True:
-                self.fail("null")
+                raise self.make_error("null")
 
     def serialize(self, attr, obj, accessor=None, **kwargs):
         """Pulls the value for the given key from the object, applies the
@@ -502,7 +516,7 @@ class Nested(Field):
     def _test_collection(self, value, many=False):
         many = self.many or many
         if many and not utils.is_collection(value):
-            self.fail("type", input=value, type=value.__class__.__name__)
+            raise self.make_error("type", input=value, type=value.__class__.__name__)
 
     def _load(self, value, data, partial=None, many=False):
         try:
@@ -632,7 +646,7 @@ class List(Field):
 
     def _deserialize(self, value, attr, data, **kwargs):
         if not utils.is_collection(value):
-            self.fail("invalid")
+            raise self.make_error("invalid")
         # Optimize loading a list of Nested objects by calling load(many=True)
         if isinstance(self.inner, Nested) and not self.inner.many:
             return self.inner.deserialize(value, many=True, **kwargs)
@@ -714,7 +728,7 @@ class Tuple(Field):
 
     def _deserialize(self, value, attr, data, **kwargs):
         if not utils.is_collection(value):
-            self.fail("invalid")
+            raise self.make_error("invalid")
 
         self.validate_length(value)
 
@@ -752,7 +766,7 @@ class String(Field):
 
     def _deserialize(self, value, attr, data, **kwargs):
         if not isinstance(value, (str, bytes)):
-            self.fail("invalid")
+            raise self.make_error("invalid")
         try:
             return utils.ensure_text_type(value)
         except UnicodeDecodeError as error:
@@ -813,7 +827,7 @@ class Number(Field):
             return None
         # (value is True or value is False) is ~5x faster than isinstance(value, bool)
         if value is True or value is False:
-            self.fail("invalid", input=value)
+            raise self.make_error("invalid", input=value)
         try:
             return self._format_num(value)
         except (TypeError, ValueError) as error:
@@ -855,7 +869,7 @@ class Integer(Number):
                 value, numbers.Integral
             ):
                 return super()._validated(value)
-            self.fail("invalid", input=value)
+            raise self.make_error("invalid", input=value)
         return super()._validated(value)
 
 
@@ -881,7 +895,7 @@ class Float(Number):
         num = super()._validated(value)
         if self.allow_nan is False:
             if math.isnan(num) or num == float("inf") or num == float("-inf"):
-                self.fail("special")
+                raise self.make_error("special")
         return num
 
 
@@ -1045,7 +1059,7 @@ class Boolean(Field):
                     return False
             except TypeError as error:
                 raise self.make_error("invalid", input=value) from error
-        self.fail("invalid", input=value)
+        raise self.make_error("invalid", input=value)
 
 
 class DateTime(Field):
@@ -1114,7 +1128,7 @@ class DateTime(Field):
 
     def _deserialize(self, value, attr, data, **kwargs):
         if not value:  # Falsy values, e.g. '', None, [] are not valid
-            self.fail("invalid", input=value, obj_type=self.OBJ_TYPE)
+            raise self.make_error("invalid", input=value, obj_type=self.OBJ_TYPE)
         data_format = self.format or self.DEFAULT_FORMAT
         func = self.DESERIALIZATION_FUNCS.get(data_format)
         if func:
@@ -1160,7 +1174,7 @@ class NaiveDateTime(DateTime):
         ret = super()._deserialize(value, attr, data, **kwargs)
         if is_aware(ret):
             if self.timezone is None:
-                self.fail(
+                raise self.make_error(
                     "invalid_awareness",
                     awareness=self.AWARENESS,
                     obj_type=self.OBJ_TYPE,
@@ -1191,7 +1205,7 @@ class AwareDateTime(DateTime):
         ret = super()._deserialize(value, attr, data, **kwargs)
         if not is_aware(ret):
             if self.default_timezone is None:
-                self.fail(
+                raise self.make_error(
                     "invalid_awareness",
                     awareness=self.AWARENESS,
                     obj_type=self.OBJ_TYPE,
@@ -1222,7 +1236,7 @@ class Time(Field):
     def _deserialize(self, value, attr, data, **kwargs):
         """Deserialize an ISO8601-formatted time to a :class:`datetime.time` object."""
         if not value:  # falsy values are invalid
-            self.fail("invalid")
+            raise self.make_error("invalid")
         try:
             return utils.from_iso_time(value)
         except (AttributeError, TypeError, ValueError) as error:
@@ -1411,7 +1425,7 @@ class Mapping(Field):
 
     def _deserialize(self, value, attr, data, **kwargs):
         if not isinstance(value, _Mapping):
-            self.fail("invalid")
+            raise self.make_error("invalid")
         if not self.value_field and not self.key_field:
             return value
 


Test patch:
diff --git a/tests/test_fields.py b/tests/test_fields.py
index 5364b83c..b0a5a777 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -189,6 +189,13 @@ class TestErrorMessages:
     class MyField(fields.Field):
         default_error_messages = {"custom": "Custom error message."}
 
+    error_messages = [
+        ("required", "Missing data for required field."),
+        ("null", "Field may not be null."),
+        ("custom", "Custom error message."),
+        ("validator_failed", "Invalid value."),
+    ]
+
     def test_default_error_messages_get_merged_with_parent_error_messages_cstm_msg(
         self
     ):
@@ -200,28 +207,26 @@ class TestErrorMessages:
         field = self.MyField(error_messages={"passed": "Passed error message"})
         assert field.error_messages["passed"] == "Passed error message"
 
-    def test_fail(self):
+    @pytest.mark.parametrize(("key", "message"), error_messages)
+    def test_make_error(self, key, message):
         field = self.MyField()
 
-        with pytest.raises(ValidationError) as excinfo:
-            field.fail("required")
-        assert excinfo.value.args[0] == "Missing data for required field."
-
-        with pytest.raises(ValidationError) as excinfo:
-            field.fail("null")
-        assert excinfo.value.args[0] == "Field may not be null."
+        error = field.make_error(key)
+        assert error.args[0] == message
 
-        with pytest.raises(ValidationError) as excinfo:
-            field.fail("custom")
-        assert excinfo.value.args[0] == "Custom error message."
+    @pytest.mark.parametrize(("key", "message"), error_messages)
+    def test_fail(self, key, message):
+        field = self.MyField()
 
-        with pytest.raises(ValidationError) as excinfo:
-            field.fail("validator_failed")
-        assert excinfo.value.args[0] == "Invalid value."
+        with pytest.warns(DeprecationWarning):
+            try:
+                field.fail(key)
+            except ValidationError as error:
+                assert error.args[0] == message
 
+    def test_make_error_key_doesnt_exist(self):
         with pytest.raises(AssertionError) as excinfo:
-            field.fail("doesntexist")
-
+            self.MyField().make_error("doesntexist")
         assert "doesntexist" in excinfo.value.args[0]
         assert "MyField" in excinfo.value.args[0]