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

Model.objects cannot be defined using QuerySet.as_manager #1713

Closed
orsinium opened this issue Sep 15, 2023 · 5 comments
Closed

Model.objects cannot be defined using QuerySet.as_manager #1713

orsinium opened this issue Sep 15, 2023 · 5 comments
Labels
bug Something isn't working

Comments

@orsinium
Copy link

Bug report

What's wrong

class SubscriptionPromotion(BasePromotion):
    objects = SubscriptionPromotionQuerySet.as_manager()
error: Cannot override class variable (previously declared on base class "BasePromotion") with instance variable  [misc]
error: Incompatible types in assignment (expression has type "Manager[BasePromotion]", base class "BasePromotion" defined the type as "ManagerFromPromotionQuerySet[BasePromotion]")  [assignment]

How is that should be

The second error was there before but the first one is newly introduced in 4.2.4. As suggested in #1712 (comment), this code should be explicitly annotated. However, the problem is that if I annotate it like this:

objects: ClassVar[models.Manager[BasePromotion]

Then all custom attributes defined in the SubscriptionPromotionQuerySet won't be available in SubscriptionPromotion.objects.

In other words, before django-stubs-ext used to construct on the fly the return type of QuerySet.as_manager as a mix of Manager and custom methods but now I can't use it because I have to add explicit annotations, and there is no explicit annotation for this dynamically constructed type.

System information

  • OS: linux
  • python version: 3.8
  • django version: 4.2.4
  • mypy version: 1.5.1
  • django-stubs version: 4.2.4
  • django-stubs-ext version: 4.2.2
@orsinium orsinium added the bug Something isn't working label Sep 15, 2023
@orsinium
Copy link
Author

We've been using django-stubs for more than a year, and that's the hardest upgrade we had so far, TBH. Automatic inference of objects was the main selling point for me to use the django-stubs-ext in addition to just django-stubs.

@flaeppe
Copy link
Member

flaeppe commented Sep 15, 2023

I've noted the problem of inheritance with a base that has managers and such too. I've started working out some stuff on it here: #1699

However, noting the type you have on your base's manager; Manager[BasePromotion], I would suggest using ._default_manager attribute where you use your BasePromotion, to get around this for now.

Since ._default_manager aligns perfectly with Manager[BasePromotion] (as long as Manager here is django.db.models.manager.Manager):

class ModelBase(type):
@property
def _default_manager(cls: type[_Self]) -> BaseManager[_Self]: ... # type: ignore[misc]

Also note that Manager has nothing in addition to BaseManager. You can verify it by looking at the source:

https://github.com/django/django/blob/814e7bc22062eeae4be9f189e89027e28d5dd290/django/db/models/manager.py#L176-L177

I think this should allow you to avoid having to declare the objects: ClassVar[...] for every subclass of your base

@flaeppe
Copy link
Member

flaeppe commented Sep 15, 2023

To clarify I meant to remove any objects: Manager[BasePromotion] from your BasePromotion model and then use ._default_manager instead of objects whenever the type is type[BasePromotion].

@orsinium
Copy link
Author

Thank you, I'll give it a try on Monday and report back here.

@orsinium
Copy link
Author

I think now I get what you mean. So, I solved the problem by removing the objects declaration from the base model and copy-pasting it into every submodel.

Before:

class BasePromotion(models.Model):
    objects = PromotionQuerySet.as_manager()

class Promotion(BasePromotion):
    pass

class SubscriptionPromotion(BasePromotion):
    objects = SubscriptionPromotionQuerySet.as_manager()

After:

class BasePromotion(models.Model):
    pass

class Promotion(BasePromotion):
    objects = PromotionQuerySet.as_manager()

class SubscriptionPromotion(BasePromotion):
    objects = SubscriptionPromotionQuerySet.as_manager()

It's not perfect, especially if there are lots of submodels, but should do for now.

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