load accepts Sequence rather than Iterable (rejects generators)

[sloria]

addresses #1898

from marshmallow import Schema, fields, validates_schema


class MySchema(Schema):
    fld = fields.String()

    @validates_schema
    def _validate(self, data, **kwargs):
        print("schema validator called")


MySchema().load(({"fld": "abc"}, {"fld": "def"}), many=True)
# schema validator called
# schema validator called


def gen():
    yield from ({"fld": "abc"}, {"fld": "def"})


MySchema().load(gen(), many=True)
# Before this change, silently fails to call schema validator
# After this change, an error is raised:
# marshmallow.exceptions.ValidationError: {"_schema": ["Invalid input type."]}

Caveat: for some reason, mypy doesn't error when passing a generator. it seems that generators are covariant with Sequence and are therefore equivalent for type-checking purposes. not sure if there is a better way to handle this.

def gen():
    yield from ({"fld": "abc"}, {"fld": "def"})


assert isinstance(gen(), Sequence) is False
MySchema().load(gen(), many=True)  # mypy accepts this

[sloria]

ah, looks like this breaks handling of sets. will investigate later

update: fixed

[sloria]

this is ready for review. i thought this was a breaking change since it changes what we're accepting as valid input. but one could argue this is bugfix, since non-sequence inputs are silently failing to call schema validators. should this be backported to v3?

[lafrech]

i thought this was a breaking change since it changes what we're accepting as valid input. but one could argue this is bugfix, since non-sequence inputs are silently failing to call schema validators. should this be backported to v3?

Arguable. I don't really mind either way, but considering the move to v4 should be easy, I'm tempted to keep things as is in v3 and expect users to move fast to v4.

Did we consider actually supporting generators? Is the rationale that this is costly to do in marshmallow so we prefer users unpack their generators themselves?

[sloria]

Did we consider actually supporting generators? Is the rationale that this is costly to do in marshmallow so we prefer users unpack their generators themselves?

we've never explicitly supported generators. the immediate rationale for this is that handling generators is currently broken but it fails silently (schema validators aren't called)

[lafrech]

Yes, I got that.

Just wondering if we could solve this the other way by actually supporting the use case, and at what cost.

This was investigated by @sirosen in #1898 (comment) and I don't think the downside he points to (post_load receives a list) is really an issue.

[sloria]

gotcha. i'd rather avoid adding another runtime type-check, for the reason sirosen pointed out (pre_load wouldn't actually receive the "raw" input, which is pretty unintuitive) as well as the performance hit.