Repository: marshmallow-code/marshmallow

Issue:
RFC: Change the way we store metadata?
Users are often bit by the fact that fields store arbitrary keyword arguments as metadata. See https://github.com/marshmallow-code/marshmallow/issues/683.

> ...The reasons we use **kwargs instead of e.g. `metadata=` are mostly historical. The original decision was that storing kwargs 1) was more concise and 2) saved us from having to come up with an appropriate name... "metadata" didn't seem right because there are use cases where the things your storing aren't really metadata. At this point, it's not worth breaking the API.

> Not the best reasons, but I think it's not terrible. We've discussed adding a [whitelist of metadata keys](https://github.com/marshmallow-code/marshmallow/issues/683#issuecomment-385113845) in the past, but we decided it wasn't worth the added API surface.

_Originally posted by @sloria in https://github.com/marshmallow-code/marshmallow/issues/779#issuecomment-522283135_

Possible solutions:

1. Use `metadata=`.
2. Specify a whitelist of allowed metadata arguments.

Feedback welcome!

Discussion:
lafrech: >  Is doing the shallow-copy with dict(metadata) the right thing? Should we maybe do nothing with the input?

Yes. As long as we update with the extra kwargs. We don't want to modify the input dict.

Patch:
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 818dfd44..ab5cc824 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -1,6 +1,15 @@
 Changelog
 ---------
 
+3.10.0 (Unreleased)
+*******************
+
+Deprecations:
+
+- Passing field metadata via keyword arguments is deprecated and will be
+  removed in marshmallow 4 (:issue:`1350`). Use the explicit `metadata=...`
+  argument instead.
+
 3.9.1 (2020-11-07)
 ******************
 
diff --git a/docs/extending.rst b/docs/extending.rst
index c6220538..f240d75e 100644
--- a/docs/extending.rst
+++ b/docs/extending.rst
@@ -160,10 +160,12 @@ In summary, the processing pipeline for deserialization is as follows:
 1. ``@pre_load(pass_many=True)`` methods
 2. ``@pre_load(pass_many=False)`` methods
 3. ``load(in_data, many)`` (validation and deserialization)
-4. ``@post_load(pass_many=True)`` methods
-5. ``@post_load(pass_many=False)`` methods
+4. ``@validates`` methods (field validators)
+5. ``@validates_schema`` methods (schema validators)
+6. ``@post_load(pass_many=True)`` methods
+7. ``@post_load(pass_many=False)`` methods
 
-The pipeline for serialization is similar, except that the ``pass_many=True`` processors are invoked *after* the ``pass_many=False`` processors.
+The pipeline for serialization is similar, except that the ``pass_many=True`` processors are invoked *after* the ``pass_many=False`` processors and there are no validators.
 
 1. ``@pre_dump(pass_many=False)`` methods
 2. ``@pre_dump(pass_many=True)`` methods
diff --git a/docs/quickstart.rst b/docs/quickstart.rst
index 367d3628..bb1bd620 100644
--- a/docs/quickstart.rst
+++ b/docs/quickstart.rst
@@ -535,25 +535,22 @@ To maintain field ordering, set the ``ordered`` option to `True`. This will inst
 
 
     class UserSchema(Schema):
-        uppername = fields.Function(lambda obj: obj.name.upper())
+        first_name = fields.String()
+        last_name = fields.String()
+        email = fields.Email()
 
         class Meta:
-            fields = ("name", "email", "created_at", "uppername")
             ordered = True
 
 
-    u = User("Charlie", "charlie@stones.com")
+    u = User("Charlie", "Stones", "charlie@stones.com")
     schema = UserSchema()
     result = schema.dump(u)
     assert isinstance(result, OrderedDict)
-    # marshmallow's pprint function maintains order
     pprint(result, indent=2)
-    # {
-    #   "name": "Charlie",
-    #   "email": "charlie@stones.com",
-    #   "created_at": "2014-10-30T08:27:48.515735+00:00",
-    #   "uppername": "CHARLIE"
-    # }
+    #  OrderedDict([('first_name', 'Charlie'),
+    #              ('last_name', 'Stones'),
+    #              ('email', 'charlie@stones.com')])
 
 Next Steps
 ----------
diff --git a/setup.py b/setup.py
index bd05da4f..bdced19b 100644
--- a/setup.py
+++ b/setup.py
@@ -14,7 +14,7 @@ EXTRAS_REQUIRE = {
         "sphinx-issues==1.2.0",
         "alabaster==0.7.12",
         "sphinx-version-warning==1.1.2",
-        "autodocsumm==0.2.1",
+        "autodocsumm==0.2.2",
     ],
 }
 EXTRAS_REQUIRE["dev"] = EXTRAS_REQUIRE["tests"] + EXTRAS_REQUIRE["lint"] + ["tox"]
diff --git a/src/marshmallow/fields.py b/src/marshmallow/fields.py
index dc73a418..340fe83e 100644
--- a/src/marshmallow/fields.py
+++ b/src/marshmallow/fields.py
@@ -101,7 +101,7 @@ class Field(FieldABC):
         its value will be present in the deserialized object. In the context of an
         HTTP API, this effectively marks the field as "read-only".
     :param dict error_messages: Overrides for `Field.default_error_messages`.
-    :param metadata: Extra arguments to be stored as metadata.
+    :param metadata: Extra information to be stored as field metadata.
 
     .. versionchanged:: 2.0.0
         Removed `error` parameter. Use ``error_messages`` instead.
@@ -160,7 +160,8 @@ class Field(FieldABC):
         load_only: bool = False,
         dump_only: bool = False,
         error_messages: typing.Optional[typing.Dict[str, str]] = None,
-        **metadata
+        metadata: typing.Optional[typing.Mapping[str, typing.Any]] = None,
+        **additional_metadata
     ) -> None:
         self.default = default
         self.attribute = attribute
@@ -187,7 +188,16 @@ class Field(FieldABC):
             raise ValueError("'missing' must not be set for required fields.")
         self.required = required
         self.missing = missing
-        self.metadata = metadata
+
+        metadata = metadata or {}
+        self.metadata = {**metadata, **additional_metadata}
+        if additional_metadata:
+            warnings.warn(
+                "Passing field metadata as a keyword arg is deprecated. Use the "
+                "explicit `metadata=...` argument instead.",
+                RemovedInMarshmallow4Warning,
+            )
+
         self._creation_index = Field._creation_index
         Field._creation_index += 1
 


Test patch:
diff --git a/tests/test_fields.py b/tests/test_fields.py
index 19abeb6b..4e5e4cc0 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -197,11 +197,27 @@ class TestMetadata:
             required=True,
             default=None,
             validate=lambda v: True,
-            description="foo",
-            widget="select",
+            metadata={"description": "foo", "widget": "select"},
         )
         assert field.metadata == {"description": "foo", "widget": "select"}
 
+    @pytest.mark.parametrize("FieldClass", ALL_FIELDS)
+    def test_field_metadata_added_in_deprecated_style_warns(self, FieldClass):  # noqa
+        # just the old style
+        with pytest.warns(DeprecationWarning):
+            field = FieldClass(description="Just a normal field.")
+            assert field.metadata["description"] == "Just a normal field."
+        # mixed styles
+        with pytest.warns(DeprecationWarning):
+            field = FieldClass(
+                required=True,
+                default=None,
+                validate=lambda v: True,
+                description="foo",
+                metadata={"widget": "select"},
+            )
+        assert field.metadata == {"description": "foo", "widget": "select"}
+
 
 class TestErrorMessages:
     class MyField(fields.Field):