Please consider using Mapping instead of Dict in ArgsMap to avoid type checker errors

[gh-andre]

Please consider using Mapping instead of Dict in ArgsMap to avoid type checker errors

[lafrech]

Thanks for reporting.

Without even trying to understand the error message, I'd say Mapping should be preferred in most cases.

Would you like to send a PR replacing Dict with Mapping in the code base?

[gh-andre]

@lafrech Thanks for responding.

Would you like to send a PR replacing Dict with Mapping in the code base?

This may be potentially detrimental because Dict is mutable and Mapping is not, so one would need to distinguish between cases where Mapping, MutableMapping or dict[X, Y] when going through the entire codebase, which would probably not the best idea for someone with no experience in this code to do.

I will give it a go and change ArgsMap to use Mapping and see if mypy generates any new errors for webargs. Will post more here later.

[gh-andre]

@lafrech
I had a look and it cannot be changed safely now because of the dependency on marshmallow. Also, the change would be quite big because current code uses ArgMap as a non-generic type, so all uses of it would have to be changed.

First, the latter, because it's simpler in nature. ArgMap would need to be changed to add Optional around Request, like this:

ArgMap = typing.Union[
    ma.Schema,
    typing.Mapping[str, typing.Union[ma.fields.Field, typing.Type[ma.fields.Field]]],
    typing.Callable[[typing.Optional[Request]], ma.Schema],
]

This is because Parser._prepare_for_parse takes Request | None. The size of the change comes from the fact that ArgMap is generic because of Request, which is TypeVar, so all uses of it would have to be changed like this:

    def _get_schema(self, argmap: ArgMap[Request], req: Request) -> ma.Schema:

That change is fine - just affects a bunch of files, but otherwise seems to be useful in general and gets rid of a bunch of mypy errors. Replacing Dict with Mapping however, isn't as simple.

This is because Schema relies on operating on various dictionary flavors, which causes this mismatch between Mapping and Dict:

src\webargs\pyramidparser.py:154: error: Argument 1 to "from_dict" of "Schema" has incompatible type "Mapping[Any, Any]"; expected "Dict[str, Union[Field, type]]" [arg-type]

This leads into the dependency project, which uses specific dictionary classes, like this code:

    def dict_class(self) -> type:
        return OrderedDict if self.ordered else dict

, and would require changes in marshmallow to return MutableMapping or some other solution if the ordered nature of the returned class has more significant meaning.

I will leave it alone. It's too big of a change for me to engage in. Please feel free to close the issue if it's not something you see changing going forward.

[sirosen]

Thanks for your research and sharing your findings! This at least gets us some sense of the scope of the change.

Our primary goal with the code locations which take ArgMap is to support dict, and the docs show usages like this:

@use_args({"foo": fields.String()}, location="query")

Since this is meant for the "super lightweight" usages, I'm very curious to hear how you ran into this. Were you using a non-dict mapping of your own creation, or something else?
Helping us understand this may change how we want to handle this case.

---

I'd prefer for us not to close this issue, at least not right away. It's not trivial, but the current annotation may or may not be considered correct, depending on your interpretation.

Are types descriptive (type is wrong, does not describe the runtime behavior correctly) or normative (type is not wrong, declares supported usages)? It's not super clear, but I'd at least like time to chew on this.

[gh-andre]

I'm using it in most basic way, similar to how you describe in Basic Usage and how typically one would use schema-based request argument with other frameworks, like Joi.

That is, distilled from class-based views and routes, it translates into this simplified sequence, which is pretty much how it is outlined in webargs docs.

self.login_args_post = {
    "email": fields.Email(required=True),
    "password": fields.Str(required=True, validate=validate.Length(min=6)),
    "refpage": fields.URL(relative=True, validate=validate.Regexp(r"..."))
}

def login_post(self) -> werkzeug.Response:
    try:
        args_form = flask_parser.parse(self.login_args_post, request, location="form")
        ... use args_form["email"], etc
    except ValidationError as error:
        ... handle error.messages
        return redirect(url_for("login", refpage=refpage))

I run mypy in the strict mode. Maybe this is what's different between errors you are seeing and what I'm reporting. For example, this command line reports about 240 errors for webargs. Some are easier to fix, like missing type parameters from Mapping, some seem trickier, like those leading into marshmallow and meta classes.

mypy --strict --check-untyped-defs --strict-equality --warn-redundant-casts --warn-incomplete-stub --warn-unreachable --install-types --non-interactive src/webargs

I currently worked it around in my code by adding explicit type to all schema definitions, which somewhat takes away from that lightweight simplicity you mentioned and couples it with the current definition of ArgMap.

self.login_args_post: Dict[str, Union[fields.Field, Type[fields.Field]]] = {
    "email": fields.Email(required=True),
    "password": fields.Str(required=True, validate=validate.Length(min=6)),
    "refpage": fields.URL(relative=True, validate=validate.Regexp(r"..."))
}

Thanks for considering it. Much appreciated.

[sirosen]

This was one of the points addressed in the typing improvements in v8.3.0, which we just released the other day.
ArgMap now accepts Mapping and we've cleaned up some other typing issues which were making that change difficult.

Thanks for filing this and pushing us to improve the annotations! Please let us know if there are any ongoing typing issues -- related to this one or otherwise -- in 8.3.0+

[gh-andre]

@sirosen Thank you!