Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CallbackDict in datastructures.pyi is not utilizing a bound TypeVar #2235

Closed
Purg opened this issue Sep 16, 2021 · 2 comments
Closed

CallbackDict in datastructures.pyi is not utilizing a bound TypeVar #2235

Purg opened this issue Sep 16, 2021 · 2 comments

Comments

@Purg
Copy link

Purg commented Sep 16, 2021

When creating a derived class of CallbackDict, I am unable to pass an on_update function annotated with the derived type and pass type checks with mypy or pyright.
The following is the derived class in question and associated on_update function (which has to be separate from the class to pass type checks due to a seeming quirk of UpdateDictMixin but that's a separate issue):

from flask.sessions import SessionMixin
from typing import Union, Iterable, Tuple, Mapping, Optional, Any
from werkzeug.datastructures import CallbackDict


class MongoSession(CallbackDict, SessionMixin):

    def __init__(
        self,
        initial: Union[Mapping, Iterable[Tuple[Any, Any]], None] = None,
        sid: Optional[str] = None
    ):
        super(MongoSession, self).__init__(initial, on_update)
        self.sid = sid
        self.modified = False


def on_update(self: MongoSession) -> None:
    self.modified = True

Currently, mypy type checking will raise the following error here:

.../mongo_sessions.py:32: error: Argument 2 to "__init__" of "CallbackDict" has incompatible type "Callable[[MongoSession], None]"; expected "Optional[Callable[[CallbackDict[Any, Any]], None]]"

This is due to the specific use of CallbackDict in the datastructures.pyi file for the on_update parameter:

class CallbackDict(UpdateDictMixin[K, V], Dict[K, V]):
    def __init__(
        self,
        initial: Optional[Union[Mapping[K, V], Iterable[Tuple[K, V]]]] = None,
        on_update: Optional[Callable[[CallbackDict], None]] = None,
    ) -> None: ...

This form of typing is interpreted as invariant, meaning that I have to pass in a function that specifically takes a CallbackDict and not a derived type.
In such a case I would not be able to modify properties on the derived class in on_update without incurring other typing errors (e.g. modified is not a property on CallbackDict nor UpdateDictMixin, but is in my derived class).

I would expect that I should be able to pass in an on_update function that is annotated to accept a derived class to perform more specific updates on.

This could be addressed by updating the datastructure.pyi file to use a _CD = TypeVar("_CD", bound="CallbackDict") instead of a concrete, invariant type annotation:

_CD = TypeVar("_CD", bound="CallbackDict")

class CallbackDict(UpdateDictMixin[K, V], Dict[K, V]):
    def __init__(
        self,
        initial: Optional[Union[Mapping[K, V], Iterable[Tuple[K, V]]]] = None,
        on_update: Optional[Callable[[_CD], None]] = None,
    ) -> None: ...

I have tested this locally and it shows correctly rejecting a non-CallbackDict derived type annotation, but does accept derived types like the above MongoSession.

Environment:

  • Python version: 3.6.8
  • Werkzeug version: 2.0.1
@davidism
Copy link
Member

Happy to review a PR.

@Purg
Copy link
Author

Purg commented Sep 28, 2021

I started to make a PR and, after finding that I broke a couple unit tests, found that a specific pattern was used for passing to the on_update parameter. I have adjusted my use to:

from flask.sessions import SessionMixin
from typing import Union, Iterable, Tuple, Mapping, Optional, Any
from werkzeug.datastructures import CallbackDict


class MongoSession(CallbackDict, SessionMixin):

    def __init__(
        self,
        initial: Union[Mapping, Iterable[Tuple[Any, Any]], None] = None,
        sid: Optional[str] = None
    ):
        def on_update(_: Mapping):
            self.modified = True

        super(MongoSession, self).__init__(initial, on_update)
        self.sid = sid
        self.modified = False

My original modification attempt failed since I missed considering the above use-case, or the use of CallbackDict as a stand-alone wrapper for a mapping and associated callback. It's possible that there is room to "fix" the PYI by changing the use of CallbackDict to a bound contra-variant type, but I am unblocked now.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Oct 13, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants