marshmallow icon indicating copy to clipboard operation
marshmallow copied to clipboard

Schema fields that use a reserved name inherit wrong type annotations

Open OJFord opened this issue 3 years ago • 5 comments

In Schema.__init__, some non-_ prefixed attributes are assigned to:

https://github.com/marshmallow-code/marshmallow/blob/fa6c7379468f59d4568e29cbbeb06b797d656215/src/marshmallow/schema.py#L385-L399

We use fields (itself), and this isn't a problem so I assume there's some automatic renaming and then attribute="fields" resetting going on.

The issue is that mypy will complain:

expression has type "Integer", base class "Schema" defined the type as "Dict[str, Field]"

due to the type annotation above.

I initially thought it was to do with the name 'fields' and 'marshmallow.fields' specifically, but you can verify it's due to the annotation by trying to assign just a str, for example, or using 'load_fields' (also annotated):

import marshmallow.Schema


class Test(marshmallow.Schema):
    load_fields = "mypy won't like this"

test.py:5: error: Incompatible types in assignment (expression has type "str", base class "Schema" defined the type as "Dict[str, Field]")

OJFord avatar Nov 26 '20 12:11 OJFord

I think the only way to solve this might be a mypy plugin. Fields defined as class attrs are handled differently from instance attrs by the SchemaMeta metaclass. Several other libraries which use special class attrs (sqlalchemy and attrs come to mind) require plugins, and mypy can't handle arbitrary metaclass behaviors.

sirosen avatar Mar 14 '22 12:03 sirosen

so what is a useable workaround?

in my case i defined field: context = fields.String(required=False)

and get mypy error:

error: Incompatible types in assignment (expression has type "String", base class "Schema" defined the type as "dict[Any, Any]") [assignment]

because marshmallow base class "Schema" in its init() actually defined context as: context: dict | None = None,

the code works fine, but mypy reports it as error and won't let me proceed. if it was a warning i'd just disable it.

tester-sn avatar Feb 27 '24 18:02 tester-sn

I think the only way to solve this might be a mypy plugin.

Wouldn't _ or even __ prefixing them be a pythonic way to solve it?

I mean it would still happen of course, just probably not be a problem because it would only happen if you were reusing something 'private', and so then when it threw the error you'd more likely think Yeah ok fair enough.

@tester-sn

so what is a useable workaround? in my case i defined field:

context = fields.String(required=False)

You can do:

_context = fields.String(required=False, attribute="context", data_key="context")

OJFord avatar Feb 28 '24 11:02 OJFord

I think the only way to solve this might be a mypy plugin.

Wouldn't _ or even __ prefixing them be a pythonic way to solve it?

I mean it would still happen of course, just probably not be a problem because it would only happen if you were reusing something 'private', and so then when it threw the error you'd more likely think Yeah ok fair enough.

There might be some redesign angle similar to that which would work.

The trouble is that these various attributes are the public interface for a Schema. For example, the following is valid and supported usage:

foo = MySchema(exclude=("alpha", "beta"))
if "alpha" in foo.exclude:
    ...

marshmallow, at least with its current design, just "isn't intelligible to type checkers" because it has a core metaclass which moves things around.

So the choices are to make type checkers understand it (hey, that approach works for attrs!) either with a plugin or with native type checker support OR for marshmallow to change. If it changes, I'm not totally sure how, it's v4 we're talking about, etc etc.

sirosen avatar Feb 28 '24 15:02 sirosen

@OJFord

You can do:

_context = fields.String(required=False, attribute="context", data_key="context")

this is excellent, sir, thank you! this is my chosen answer)

tester-sn avatar Feb 28 '24 15:02 tester-sn