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

Mypy flags unreachable on pre-save and checking for instance ID #2014

Open
sshishov opened this issue Mar 18, 2024 · 2 comments
Open

Mypy flags unreachable on pre-save and checking for instance ID #2014

sshishov opened this issue Mar 18, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@sshishov
Copy link
Contributor

Bug report

We are implementing pre-save of some models and there usually we check if the instance is being created or not by doing

if instance.id is None:
    ...  # creating the instance logic

What's wrong

import typing as t

from django.db import models as django_models
from django import dispatch as django_dispatch

@django_dispatch.receiver(django_models.signals.pre_save, sender=my_app.MyModel)
def project_pre_save(sender: type[my_app.MyModel], instance: my_app.MyModel, **kwargs: t.Any) -> None:
    if instance.id is None:
        add_something_to_instance(instance)  # <-- Statement is unreachable  [unreachable]
    ...

In this case mypy flagging this with the following error indicated above

How is that should be

On pre-save instance.id should be either the Field or None as it can be None in case of creation of the object.
Do not know how to properly implement it though...

System information

  • OS: MacOS / Linux
  • python version: 3.11.5
  • django version: 3.2.23
  • mypy version: 1.9.0
  • django-stubs version: 4.2.7
  • django-stubs-ext version: 4.2.7
@sshishov sshishov added the bug Something isn't working label Mar 18, 2024
@sobolevn
Copy link
Member

Yes, this is the trade-off we had to make. We can add fake UnsavedModel subclass (to stubs and ext) of Model with id set to T | None

What do you think?

@sshishov
Copy link
Contributor Author

Hi @sobolevn

Thank you for the report, I just have a question how this UnsavedModel will be used by people. If it will require a lot of "hand-writing" or workarounds then better to keep it as is and use type: ignore[<specific-error>] instead.

Will it be like a wrapper? Like we have WithAnnotations for queryset/model?

If it will be like a wrapper, then I am "supporting" this approach!

Thanks for your valuable time! ⏲️

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

No branches or pull requests

2 participants