Skip to content

Using TypedDict for templates rendering #355

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

Closed
dmerejkowsky opened this issue Apr 7, 2020 · 7 comments · Fixed by #376
Closed

Using TypedDict for templates rendering #355

dmerejkowsky opened this issue Apr 7, 2020 · 7 comments · Fixed by #376
Labels
bug Something isn't working

Comments

@dmerejkowsky
Copy link

Bug report

What's wrong

So, not sure if it's a bug or not - maybe what I'm trying to achieve is
outside the scope of the typeddjango project. Anyway, here goes.

I have a view_index() view that delegates building the context used
for template rendering to a IndexPresenter class with a get_context()
method:

Here are the relevant snippets (tell me if you need more):

from tying import TypedDict
from models import User, Category

IndexContext = TypedDict("IndexContext", {"user": User, "categories": List[Category]})

class IndexPresenter:
    def __init__(self, user: User):
        self.user = user

    def get_context(self) -> IndexContext:
        categories = Category.objects.all().order_by("name")
        return {"user": self.user, "categories": list(categories)}

@login_required()
def index(request: HttpRequest) -> HttpResponse:
    user = request.user
    presenter = IndexPresenter(user)
    context = presenter.get_context()
    return render(request, "audiotheque/index.pug", context)

You can see I'm using the new TypedDict from Python 3.8

The problem is that mypy complains about the last line:

Argument 3 to "render" has incompatible type  "IndexContext";
expected "Optional[Dict[str, Any]]"

How is that should be

I guess that mypy should see that a TypedDict is compatible with a Dict[str, Any] somehow?

System information

  • OS:
  • python version: 3.8.2
  • django version: 3.0.3
  • mypy version: 0.770
  • django-stubs version: 1.5.0
@dmerejkowsky dmerejkowsky added the bug Something isn't working label Apr 7, 2020
@dmerejkowsky
Copy link
Author

Oh, and one more thing - I don't really mind if the type check does not work "all the way through" - I think that the annotation of the returned dict already serves quite nicely as "executable documentation" for the templates authors :)

@kszmigiel
Copy link
Member

@dmerejkowsky Hello! I've investigated your code, and I think that's not really a problem with Mypy. The reason is this part:

TypedDict("IndexContext", {"user": User, "categories": List[Category]})

Here you define "user" as User instance, while render() function expects context to be Dict[str, Any] or none.

Also, it's weird that Mypy hasn't warned you about doing "user": self.user. According to AbstractBaseUser class in Django self.user returns a string:

def __str__(self):
        return self.get_username()

So, changing "user": User to "user": str should do the job. Unfortunately I had no time to check it, so please let me know if it works :)

@dmerejkowsky
Copy link
Author

dmerejkowsky commented May 30, 2020

I don't think it's an issue in django-stubs actually

This also fails:

from typing import Any, Dict, TypedDict


IndexContext = TypedDict("IndexContext", {"user": str})


def get_context() -> IndexContext:
    return {"user": "john"}


def render(context: Dict[str, Any]) -> None:
    print(context)


def main() -> None:
    context = get_context()
    render(context)

I've aked on mypy bug tracker: python/mypy#8923

@kszmigiel
Copy link
Member

@dmerejkowsky Now I see that my first comment was actually pretty dummy, so sorry for that. I haven't checked Mypy docs on TypedDict, and it was pretty late at night when I was writing it iirc.

So, according to this Mypy issue it's done on purpose, and probably there's no convenient way to make TypedDict work when Dict is expected (probably there exists some workaround, who knows). The solution to this would be to change type annotation for context in stubs from Dict to Mapping. I'd like to ask @sobolevn for opinion on this.

@dmerejkowsky
Copy link
Author

Now I see that my first comment was actually pretty dummy

No worries - it happens. Also, it forced me to write a simpler example, so this was actually a win :)

The solution to this would be to change type annotation for context in stubs from Dict to Mapping. I'd like to ask @sobolevn for an opinion on this.

I agree. It looks like a good idea from where I'm standing but I'm not aware of all the implications.

Thanks for your help anyway.

@sobolevn
Copy link
Member

Yes, we can try Mapping!

@kszmigiel kszmigiel mentioned this issue Jun 1, 2020
@dmerejkowsky
Copy link
Author

Thank you - keep up the good work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Development

Successfully merging a pull request may close this issue.

3 participants