Repository: marshmallow-code/marshmallow
Pull request #1627 (2020-07-08 18:36:41)

Fix usage of intersection/union for OrderedSet in Nested field
Fix a bug where a `Nested` field initialized with an instance of a schema (instead of a class) would fail to set the `only` and/or `exclude` properties when marshaling, due to trying to use methods that do not exist in `OrderedSet`.

juannorris: Hi @sloria. First time contributing to the project, please let me know if I also need to create an issue with details.

I could have changed the code for the `Nested` field where it does `set_class.intersection` for `set_class &`, but I thought mimicking the basic `set` operations would be a better approach. Please let me know if you think otherwise.

Thanks for this great family of libraries btw!
sloria: Thanks for the PR!

`OrderedSet` is vendorized from Raymond Hettinger's implementation. I hesitate to have it diverge from the original implementation, as I'm not sure whether the omission of those methods was intentional or not. Can we get away without modifying `orderedset.py`?

Alternatively, you could do some research on OrderedSet's implementation to see if others have `union`, etc. implemented. I'm open to swapping out the vendorized module if improved implementations exist.
juannorris: Thanks for the quick reply.

Yes, we can get by without modifying `orderedset.py`, by changing these lines:

https://github.com/marshmallow-code/marshmallow/blob/dev/src/marshmallow/fields.py#L525
https://github.com/marshmallow-code/marshmallow/blob/dev/src/marshmallow/fields.py#L528

to use the `&` and `|` set operators.

This OrderedSet library (https://github.com/simonpercivall/orderedset/blob/master/lib/orderedset/_orderedset.pyx) has these methods already implemented and is based on that same recipe, but I'm guessing you prefer not to have a dependency.

Please let me know how should I proceed, I can quickly change the code in `fields.py` and revert the changes in `orderedset.py` if that's the way to go.
juannorris: > I think the lowest-impact change is to use `&` and `|` in the lines you linked, and cast `original` to `set_class`. What do you think?

Yes, I agree with that, I'll update the PR.
sloria: Thanks--looks good. I'll merge this when I have some a moment to give it a closer look and update the changelog and such. 
juannorris-santex: Thanks, appreciate it!
juannorris: Thanks, appreciate it!


Patch:
diff --git a/AUTHORS.rst b/AUTHORS.rst
index a7ee306b..cb67ab7f 100644
--- a/AUTHORS.rst
+++ b/AUTHORS.rst
@@ -150,3 +150,4 @@ Contributors (chronological)
 - Laurie Opperman `@EpicWink <https://github.com/EpicWink>`_
 - Ram Rachum `@cool-RR <https://github.com/cool-RR>`_
 - `@weeix <https://github.com/weeix>`_
+- Juan Norris `@juannorris <https://github.com/juannorris>`_
diff --git a/CHANGELOG.rst b/CHANGELOG.rst
index 7cab4ac8..93a6ec08 100644
--- a/CHANGELOG.rst
+++ b/CHANGELOG.rst
@@ -12,6 +12,11 @@ Support:
 
 - Document ``default_error_messages`` on field classes (:pr:`1619`). Thanks :user:`weeix`.
 
+Bug fixes:
+
+- Fix passing ``only`` and ``exclude`` to ``Nested`` with an ordered ``Schema`` (:pr:`1627`).
+  Thanks :user:`juannorris` for the PR.
+
 3.6.1 (2020-06-02)
 ******************
 
diff --git a/src/marshmallow/fields.py b/src/marshmallow/fields.py
index a233e25d..c727292b 100644
--- a/src/marshmallow/fields.py
+++ b/src/marshmallow/fields.py
@@ -522,10 +522,10 @@ class Nested(Field):
                         original = self._schema.only
                     else:  # only=None -> all fields
                         original = self._schema.fields.keys()
-                    self._schema.only = set_class(self.only).intersection(original)
+                    self._schema.only = set_class(self.only) & set_class(original)
                 if self.exclude:
                     original = self._schema.exclude
-                    self._schema.exclude = set_class(self.exclude).union(original)
+                    self._schema.exclude = set_class(self.exclude) | set_class(original)
                 self._schema._init_fields()
             else:
                 if isinstance(nested, type) and issubclass(nested, SchemaABC):


Test patch:
diff --git a/tests/test_fields.py b/tests/test_fields.py
index 8671be95..19abeb6b 100644
--- a/tests/test_fields.py
+++ b/tests/test_fields.py
@@ -277,6 +277,24 @@ class TestNestedField:
             with pytest.raises(ValidationError):
                 MySchema().load({"nested": {"x": 1}})
 
+    @pytest.mark.parametrize(
+        ("param", "fields_list"), [("only", ["foo"]), ("exclude", ["bar"])]
+    )
+    def test_ordered_instanced_nested_schema_only_and_exclude(self, param, fields_list):
+        class NestedSchema(Schema):
+            foo = fields.String()
+            bar = fields.String()
+
+            class Meta:
+                ordered = True
+
+        class MySchema(Schema):
+            nested = fields.Nested(NestedSchema(), **{param: fields_list})
+
+        assert MySchema().dump({"nested": {"foo": "baz", "bar": "bax"}}) == {
+            "nested": {"foo": "baz"}
+        }
+
 
 class TestListNested:
     @pytest.mark.parametrize("param", ("only", "exclude", "dump_only", "load_only"))