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 can't get custom manager type #1067

Open
maximsht opened this issue Jul 21, 2022 · 12 comments
Open

Mypy can't get custom manager type #1067

maximsht opened this issue Jul 21, 2022 · 12 comments
Labels
3rd party Presence of a third party dependency has been mentioned bug Something isn't working mypy-plugin Issues specific to mypy_django_plugin

Comments

@maximsht
Copy link

maximsht commented Jul 21, 2022

I have custom user model that inherits from UserProfile from django-tenant-users package. UserProfile has custom model manager. In my model I see an error from mypy:
Mypy: Could not resolve manager type for "Flow_BE.applications.users.models.User.objects"
and when i try to use this manager I receive error:
Mypy: "Manager[User]" has no attribute "create_user"

This is how models specified:

class User(UserProfile):
    """This User model replaces default user model."""

# from django-tenant-users
class UserProfile(AbstractBaseUser, PermissionsMixinFacade):

    objects = UserProfileManager()

class UserProfileManager(BaseUserManager):

    def create_user(
            self,
            email=None,
            password=None,
            is_staff=False,
            **extra_fields,
        ):...

For User.objects.create_user(...) mypy generates errors. Code works.

System information

  • OS: MacOs 12.3.1
  • python version: 3.10.4
  • django version: 4.0.6
  • mypy version: 0.971
  • django-stubs version: 1.12.0
  • django-stubs-ext version: 0.5.0
@maximsht maximsht added the bug Something isn't working label Jul 21, 2022
@sobolevn
Copy link
Member

Hi @MaximShteygervald 🙂
Nice to see you again!

We are working on better manager types in a new release, so it might be already fixed.
Can you please try master version?

@flaeppe
Copy link
Member

flaeppe commented Aug 7, 2022

I think this is related to #1023 and the fact that django-tenant-users is an untyped package.

@mvaled
Copy link
Contributor

mvaled commented Aug 16, 2022

I'm trying to update django-stubs and ext, but I'm being blocked by this issue.

We usually create a custom queryset and use as_manager. To avoid complications we use an explicit (albeit not 100% accurate, but really useful) annotation:

class AttachmentQS(QuerySet['Attachment']):
     def load(self, pk) -> 'Attachment': ...

class Attachment(Model):
     # ...
     objects: AttachmenQS = AttachmentQS.as_manager()  # type: ignore 

This has worked pretty well so far, but trying to update apparently makes our hack being ignored and I get lots of issues like:

kaiko/attachments/tasks.py:112:18: error: "Manager[Attachment]" has no attribute "load"

when calling Attachment.objects.load(...). Does django-stubs-ext overrides the explicit type hint somehow? (Is it even possible to override it)?

@mvaled
Copy link
Contributor

mvaled commented Aug 16, 2022

Hi @flaeppe,

Notice that issue is different in two regards:

  • They comment the type of instances becomes Any,
  • which allows any attribute/method in the instances.

In this case is the actual manager's type the one missing the methods in the queryset.

The line that fails is calling Attachment.objects.<some_custom_method>; but, in any case, since I put an explicit type hint, the type of Attachment.objects should not be Manager[Attachment] but the type I explicitly attached to it.

Just to test, I did the change to use from_queryset:

class AttachmentQS(QuerySet['Attachment']):
     def load(self, pk) -> 'Attachment': ...

AttachmentManager = Manager.from_queryset(AttachmentQS)

class Attachment(Model):
     # ...
     objects = AttachmentManager()

And still the same error is reported.

image

@flaeppe
Copy link
Member

flaeppe commented Aug 16, 2022

[...] since I put an explicit type hint, the type of Attachment.objects should not be Manager[Attachment] but the type I explicitly attached to it.

I fear this is overwritten by the plugin code (at least when using .from_queryset(...))

Just to test, I did the change to use from_queryset

If you got that error without an explicit annotation, there's probably a bug somewhere.

@mvaled
Copy link
Contributor

mvaled commented Aug 16, 2022

Oh! I've just realized that I did a slightly different thing:

objects = Manager.from_queryset(AttachmentQS)()

Which issues this warning:

kaiko/attachments/models.py:132:5: error: Could not resolve manager type for "kaiko.attachments.models.Attachment.objects"
kaiko/attachments/models.py:132:15: error: `.from_queryset` called from inside model class body

Extracting the manager, does solve this particular issue.

@sebastian-philipp
Copy link
Contributor

I had the very same issue with django-safedelete. My solution was a PR against upstream makinacorpus/django-safedelete#213 . Thankfully it got merged quickly.

@agalazis
Copy link

agalazis commented Sep 3, 2022

In my opinion Manager generic should accept Model, Quesryset[Model] types allowing to specify any class that extends Quesryset[Model] as the queryset

@intgr intgr added the mypy-plugin Issues specific to mypy_django_plugin label Nov 8, 2022
@flaeppe flaeppe added the 3rd party Presence of a third party dependency has been mentioned label Nov 1, 2023
@bwo
Copy link

bwo commented Nov 16, 2023

@mvaled See

* https://github.com/typeddjango/django-stubs#my-queryset-methods-are-returning-any-rather-than-my-model, and

* [Implement support for `<QuerySet>.as_manager()` #1025](https://github.com/typeddjango/django-stubs/pull/1025)

What is the first link supposed to communicate? The answer to the question is just "if you're doing this [example] or [this]" but there's no consequent: if you're doing this things … then what?

The PR in the second link has been merged, but I still get errors: first, a 'need type annotation for "objects"' error when doing query_set.as_manager() or Manager.from_queryset(query_set)(), and second, revealed types of Any when using the querysets.

@flaeppe
Copy link
Member

flaeppe commented Nov 16, 2023

What is the first link supposed to communicate? The answer to the question is just "if you're doing this [example] or [this]" but there's no consequent: if you're doing this things … then what?

I think the revision behind the link said something different than today, at the time the comment was written. Before support for <QuerySet>.as_manager() existed. But you're totally right that section doesn't really provide any answer right now, it should probably be removed.

The PR in the second link has been merged, but I still get errors: first, a 'need type annotation for "objects"' error when doing query_set.as_manager() or Manager.from_queryset(query_set)(), and second, revealed types of Any when using the querysets.

It's really hard to say anything without more context. Feel free to post an example, repro case or any other context that you think could be useful to help hunt down the cause of the issue you're seeing.

@bwo
Copy link

bwo commented Nov 16, 2023

It's really hard to say anything without more context. Feel free to post an example, repro case or any other context that you think could be useful to help hunt down the cause of the issue you're seeing.

Yeah, I eventually worked out how to do what I wanted as I attempted to repro and discovered that the actual case depends on the fact that the model in question is a PostgresPartitionedModel from django-postgres-extra—which is typed but doesn't seem to play nicely with the plugin. Would be nice, but I assume that's a big feature.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
3rd party Presence of a third party dependency has been mentioned bug Something isn't working mypy-plugin Issues specific to mypy_django_plugin
Development

No branches or pull requests

8 participants