Nesting field should validate nested schema instance

[frol]

Consider the following test cases:

class Inner(Schema):
    foo = fields.Field()
    bar = fields.Field()
    baz = fields.Field()

def test_nested_schema_instance_only_and_exclude():
    class Outer(Schema):
        inner = fields.Nested(Inner(), only=('foo', 'bar'), exclude=('bar', ))

    sch = Outer()
    data = dict(inner=dict(foo=42, bar=24, baz=242))
    result = sch.dump(data)
    assert 'foo' in result.data['inner']
    assert 'bar' not in result.data['inner']

def test_nested_schema_class_only_and_exclude():
    class Outer(Schema):
        inner = fields.Nested(Inner, only=('foo', 'bar'), exclude=('bar', ))

    sch = Outer()
    data = dict(inner=dict(foo=42, bar=24, baz=242))
    result = sch.dump(data)
    assert 'foo' in result.data['inner']
    assert 'bar' not in result.data['inner']

The only difference is Inner() vs Inner. The first test case fails as bar field will be included even though it is passed to the exclusion list of the nesting field.

Thus, when SchemaABC instance is passed we should validate many, only, exclude settings and raise an error if something is conflicting.

P.S. The second test case is taken from marshmallow/tests/test_schema.py.

[sloria]

I think validating many, only, and exclude is a good idea--it will certainly prevent incorrect usages.

[remeika]

According to the docs, this is (now) a known limitation:

When passing a Schema instance as the first argument, the instance’s exclude, only, and many attributes will be respected.

Therefore, when passing the exclude, only, or many arguments to fields.Nested, you should pass a Schema class (not an instance) as the first argument.

Is this a satisfactory state of affairs? It seems to me the following behaviors would be preferable, in descending order of preference:

1. Both Schema classes and instances have the same behavior when passed to fields.Nested
2. Passing a Schema instance to fields.Nested is not allowed and raises an exception
3. Passing a Schema instance and also exclude, only, and/or many to fields.Nested raises an exception.

As soon as I have some guidance on what should happen, I'm happy to work on this.

[sirosen]

I believe this was resolved.

I cracked open Nested to take a look at making it a warning (to convert to an error in a future version) to pass an instance. But it seems that support was added for applying only and exclude to an instance -- and even merging them via AND or OR semantics with the instance settings.

The code is maybe a little hard to understand, but the tests cover simple cases and are pretty clear.

Should I put in a PR with a doc update, or should this just be closed?