Misleading type annotations for Schema.from_dict

[SteadBytes]

The type annotation for Schema.from_dict specifies the fields parameter is of type Dict[str, Union[ma_fields.Field, type]]: https://github.com/marshmallow-code/marshmallow/blob/c972d6c27fd6a2daeb4764f3f83e542fdc9d704d/src/marshmallow/schema.py#L422

I understand that this is because one can either pass ma_fields.Field instances or a Python builtin type e.g. str or int. However, type suggests that one could pass an ma_fields.Field class which fails when an instance of the resulting Schema class is instantiated:

>>> from marshmallow import fields, Schema                                                  
>>> MySchema = Schema.from_dict({"foo": fields.Str()})                                      
>>> MySchema()                                                                              
<GeneratedSchema(many=False)>

>>> MySchema = Schema.from_dict({"foo": str})                                               
>>> MySchema()                                                                              
<GeneratedSchema(many=False)>

>>> MySchema = Schema.from_dict({"foo": fields.Str})                                        
>>> MySchema()                                                                              
Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/home/ben/.local/lib/python3.8/site-packages/marshmallow/schema.py", line 398, in __init__
    self._init_fields()
  File "/home/ben/.local/lib/python3.8/site-packages/marshmallow/schema.py", line 980, in _init_fields
    self._bind_field(field_name, field_obj)
  File "/home/ben/.local/lib/python3.8/site-packages/marshmallow/schema.py", line 1050, in _bind_field
    raise TypeError(msg) from error
TypeError: Field for "foo" must be declared as a Field instance, not a class. Did you mean "fields.String()"?

Field for "foo" must be declared as a Field instance, not a class. Did you mean "fields.String()"?

The error message is very useful and it's immediately clear what needs to be changed to resolve the error, however it would be great if this could be caught during type checking.

The simplest approach could be to replace the type annotation with Type[int], Type[str], <other supported builtin types>] however this may be too restrictive and/or cumbersome to maintain (I'm not familiar enough with all the possible use cases to be certain).

Happy to help with implementation if this is something that is wanted, otherwise I appreciate your time taken to consider the issue anyway :smiley:

[deckar01]

The value of the argument is being validated. I don't think that is the responsibility of the type checker.

https://github.com/marshmallow-code/marshmallow/blob/c972d6c27fd6a2daeb4764f3f83e542fdc9d704d/src/marshmallow/schema.py#L309

The value is used to key into a dict that is also annotated with type.

[SteadBytes]

Ah yes that's helpful - that actually shows the set of types that are valid so a more precise annotation could be a Union of those types. I'd argue that is the responsibility of the type checker; reduce the set of inputs towards the set of valid inputs e.g. in this case type encompasses any type object, whereas this function really only works on a small number of types (those in the dict you linked).

As I said, it's not a major issue so I'm happy to close this if it's deemed not important :)

[sirosen]

Looking at open typing issues, I think this one should be closed.

If we could gracefully reject Schema.from_dict({"x": fields.Str}) without causing other issues, I'd be in favor of making changes. But I don't think there's a good solution.

I can subclass Schema to extend the type mapping:

class BaseSchema(Schema):
    TYPE_MAPPING = {
        Foo: FooField,
        **Schema.TYPE_MAPPING,
    }

This isn't documented as far as I know, but it is valid/allowed. If you annotate with a specific type[x | y | ...] value, it would no longer be accurate.

Plus, there's a maintenance burden associated with this because we'd have to keep the types synchronized manually. If the resulting type were 100% accurate, I could make the case for it, but given that it isn't quite right, I don't think it is justified as a change.