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

error: Couldn't resolve related manager for relation on python3.11 and django3.2 #1354

Closed
marsha97 opened this issue Jan 31, 2023 · 19 comments
Closed
Labels
bug Something isn't working mypy-plugin Issues specific to mypy_django_plugin

Comments

@marsha97
Copy link

Bug report

What's wrong

I got Couldn't resolve related manager for relation error when using python3.11 and django3.2.
So the model looks like

# dummy/apps/foos/models.py

class Foo(models.Model):
    ....
# dummy/apps/bars/models.py

class BarQueryset(models.Queryset):
    ...

class Bar(models.Model):
    ...
   foo = models.Foreignkey('foos.Foo', related_name='bars')
   objects = models.Manager.from_queryset(BarQueryset)()

I will get the following error:
dummy/apps/foos/models.py:###: error: Couldn't resolve related manager for relation 'bars' (from dummy.apps.bars.models.Bar.bars.Bar.foo). [django-manager-missing]

It seems only happen when the reverse foreign key has a custom objects like Bar above...
And may be related to #1285

I have to use # type: ignore for now to silent this...

How is that should be

Should have no problem. I tried to downgrade python back to python3.9, mypy==0.812, and django-stubs==1.8.0 and it has no issue.

System information

  • OS: ubuntu 20
  • python version: 3.11
  • django version: 3.2.16
  • mypy version: 0.991
  • django-stubs version: 1.14.0
  • django-stubs-ext version: 0.7.0
@marsha97 marsha97 added the bug Something isn't working label Jan 31, 2023
@flaeppe flaeppe added the mypy-plugin Issues specific to mypy_django_plugin label Jan 31, 2023
@ljodal
Copy link
Contributor

ljodal commented Feb 6, 2023

I tried to downgrade python back to python3.9, mypy==0.812, and django-stubs==1.8.0 and it has no issue.

Are you sure you're getting typing on that setup? I introduced this error message a while back, but it was only to surface issues that were previously silently ignored.

Basically, this error means that the plugin was not able to statically figure out what class the manager would be. Typically that means you're doing something in your code that's not statically checkable (or not supported by the plugin). Are you able to create a minimal example that reproduces the issue?

@marsha97
Copy link
Author

marsha97 commented Feb 6, 2023

I tried to downgrade python back to python3.9, mypy==0.812, and django-stubs==1.8.0 and it has no issue.

Are you sure you're getting typing on that setup? I introduced this error message a while back, but it was only to surface issues that were previously silently ignored.

Basically, this error means that the plugin was not able to statically figure out what class the manager would be. Typically that means you're doing something in your code that's not statically checkable (or not supported by the plugin). Are you able to create a minimal example that reproduces the issue?

ooh, right. I noticed something else, this will happen if the reverse FK class inherit from this 3rd party class https://github.com/jazzband/django-model-utils/blob/cced1c7aeaf97fa6ee5c7e44c8ea827ed10b2da5/model_utils/models.py#L19
Still has no idea how to resolve this

so the simple code will looks like:

# apps/foos/models.py

from django.db import models


class Foo(models.Model):
    field1 = models.PositiveSmallIntegerField()
# apps/bars/models.py

from django.db import models
from model_utils.models import TimeStampedModel


class BarQueryset(models.QuerySet):
    def get_active_qs(self) -> models.QuerySet:
        return self.filter(is_active=True)


class Bar(TimeStampedModel):
    foo = models.ForeignKey("foos.Foo", related_name="bars", on_delete=models.CASCADE)
    is_active = models.BooleanField(default=False)
    objects = models.Manager.from_queryset(BarQueryset)()

this will results in:
dummy/apps/foos/models.py:4: error: Couldn't resolve related manager for relation 'bars' (from dummy.apps.bars.models.Bar.bars.Bar.foo). [django-manager-missing]

removing the custom objects attribute will produce the same error.

but, it will be fine if I changed the Bar model into something like this:

from django.db import models


class BarQueryset(models.QuerySet):
    def get_active_qs(self) -> models.QuerySet:
        return self.filter(is_active=True)


class BaseBar(models.Model):
    name = models.CharField(max_length=255)


class Bar(BaseBar):
    foo = models.ForeignKey("foos.Foo", related_name="bars", on_delete=models.CASCADE)
    is_active = models.BooleanField(default=False)
    objects = models.Manager.from_queryset(BarQueryset)()

@ljodal
Copy link
Contributor

ljodal commented Feb 6, 2023

Ah, alright. I guess the error message could be improved here, but basically it appears that TimeStampedModel has its managers dynamically added, which is hard/impossible to statically type check.

@intgr
Copy link
Collaborator

intgr commented Feb 6, 2023

I didn't look into this in detail, but maybe it helps to add manual type hints to the Model class for these manager attributes?

@baileywickham
Copy link

I was able to fix this issue by annotating the related name. In this case it would be

class Foo:
    field1 = models.PositiveSmallIntegerField()
    bars: "QuerySet[Bar]"
    # or if related_name isn't set:
    bar_set: "QuerySet[Bar]"

@ibadarrohman
Copy link

ibadarrohman commented Jul 26, 2023

I was able to fix this issue by annotating the related name. In this case it would be

class Foo:
    field1 = models.PositiveSmallIntegerField()
    bars: "QuerySet[Bar]"
    # or if related_name isn't set:
    bar_set: "QuerySet[Bar]"

Annotating with RelatedManager[Bar] from django.db.models.manager.RelatedManager also works.

@flaeppe
Copy link
Member

flaeppe commented Sep 24, 2023

I deem that either any of the suggested approaches with explicit annotation is a resolution or if there's 3rd party code involved(ref: #1354 (comment)) this is a duplicate of #1023.

Feel free to correct me if I'm wrong.

@flaeppe flaeppe closed this as not planned Won't fix, can't repro, duplicate, stale Sep 24, 2023
@browser-bug
Copy link

I was able to fix this issue by annotating the related name. In this case it would be

class Foo:
    field1 = models.PositiveSmallIntegerField()
    bars: "QuerySet[Bar]"
    # or if related_name isn't set:
    bar_set: "QuerySet[Bar]"

Annotating with RelatedManager[Bar] from django.db.models.manager.RelatedManager also works.

This would throw Module "django.db.models.manager" has no attribute "RelatedManager" , correct me if I'm wrong.

@sjdemartini
Copy link

Yeah, annotating with RelatedManager[Bar] from django.db.models.manager.RelatedManager does not work and results in a mypy error of Module "django.db.models.manager" has no attribute "RelatedManager" [attr-defined].

The other suggestion of:

class Foo:
    field1 = models.PositiveSmallIntegerField()
    bars: "QuerySet[Bar]"
    # or if related_name isn't set:
    bar_set: "QuerySet[Bar]"

seems to somewhat work, but in practice I've had to do the following, or I get error: Name "Bar" is not defined [name-defined]:

from typing import TYPE_CHECKING

class Foo:
    field1 = models.PositiveSmallIntegerField()

    if TYPE_CHECKING:
        from apps.bars.models import Bar  # Avoid circular imports
        bars: QuerySet[Bar]

However, this does require a bunch of manual effort and redundancy. (For instance, in my app, the User model has a custom objects definition, and I need to define these reverse relations for ~20 models that have a FK to User.) I also assume QuerySet[Bar] is an imprecise annotation compared to a proper "related manager" (e.g. where you could use add, remove, clear, or set). It would be nice if these reverse relation types could be inferred more directly by django-stubs.

Side note: This issue has surfaced for me both for models that have an objects definition, as described in the original post at the top, and for some relations that come from related_query_name of GenericForeignKey. 🤷

Is there any possibility this could be re-opened with a goal of having this resolution be automatic and more precise in django-stubs? I've sadly stuck with django-stubs 4.2.0 still due to this change in behavior (and a different ManyToMany Need type annotation for "..." issue that newer django-stubs introduced).

@flaeppe
Copy link
Member

flaeppe commented Jul 19, 2024

I'm going to go with my gut feeling here and suggest that;


Is there any possibility this could be re-opened with a goal of having this resolution be automatic and more precise in django-stubs?

I would claim that newer version(s) have made this more precise. If you're having troubles with newer version(s) and my suggestions above don't help: please open a new issue describing the problem(s) you're running in to with the newer versions and we'll try to help you out.

@sjdemartini
Copy link

sjdemartini commented Jul 19, 2024

Thanks @flaeppe for replying. I hadn't noticed that caveat from those release notes about the change in import path for that type; good to know. Regardless, it still feels like a lot of manual effort to define every reverse relation's type (especially since it has to be done in a TYPE_CHECKING block to allow for referencing the models without circular imports). This has not been necessary in my project until upgrading django-stubs beyond 4.2 (despite this GH issue being older than that)—is it not possible for django-stubs to do this reverse-relation typing automatically? (The precision I was referring to was for "related manager" as opposed to "queryset", so that would be solved with the different django_stubs_ext import you suggest, but doesn't resolve the need to manually add all of the type annotations.)

Re the ManyToManyField problem, I had seen the GH issue you mentioned, and unfortunately in my case, it occurs for fields that directly reference a model class (not using strings), and also for ones that don't specify a through value (since I know that can lead to similar issues, and which means per your comment that I can't easily add a type annotation myself). If I have some significant spare time, I can try to create a smaller example case to demonstrate the problem and open an issue for it.

@flaeppe
Copy link
Member

flaeppe commented Jul 19, 2024

I don't know what type checker you're using. But if you're using mypy and have configured the mypy plugin django-stubs offers, it should indeed add attributes for e.g. related managers automatically. I can't say from what version and it has been improved over time. So I would very much suggest to try to upgrade to our latest version to get more accurate type hints.

I'm afraid you'd have to create a new issue for the problem with ManyToManyField, I can't recall that we have any open issues related to not using string references for the to model that yields the var-annotated error you mentioned.

@sjdemartini
Copy link

sjdemartini commented Jul 19, 2024

Strange, okay, I use mypy and the django-stubs mypy_django_plugin, and have since 2020. Based on what you're saying where related managers should be added automatically (as they apparently have been for me up through django-stubs 4.2.0), it sounds like a bug then in a newer django-stubs version. As of this morning, I had tried the latest django-stubs 5.0.2 with mypy 1.10.0, and still had the above problem (where I get error: Couldn't resolve related manager for relation '...' for many reverse relations in my project).

I tried installing other versions, and this reverse relations problem is introduced as of django-stubs 4.2.4 (tested with mypy 1.5.1). django-stubs 4.2.3 and mypy 1.4.1 do not present this problem.

If I'm able, I can spend some time putting together an example that demonstrates both of the problems I've described above (reverse relations leading to "Couldn't resolve related manager", and ManyToManyFields leading to "Need type annotation"). The latter seems to happen as of django-stubs 4.2.5 (does not occur in 4.2.4).

@gone
Copy link

gone commented Aug 2, 2024

Unfortuantly I'm able to replicate, including introducing the bug when moving between 4.2.3 -> 4.2.4

@nagromLobo
Copy link

any updates here?

@danjac
Copy link

danjac commented Sep 14, 2024

Not sure why this was closed, but I have run into the same issue, even trying the django_stubs_ext.db.models.manager.RelatedManager workaround suggested (Python 3.12, django 5.1, django-stubs 5.0.4).

This is with models subclassing TimeStampedModel : it seems the only solution is to just not use this base class?

@ivan-klass
Copy link

ivan-klass commented Sep 17, 2024

@danjac I have exactly the same issue.

For some reason, it could be worked around by changing

class MyModel(TimeStampedModel):
     ...

to

class _TSM(models.Models if TYPE_CHECKING else TimeStampedModel):
    class Meta:
        abstract = True

class MyModel(_TSM):
     ...

but that doesn't seem reasonable, I would prefer to wait until this is fixed (or reverted, the only thing I wanted from 5.0.4 is mypy 1.11 support).

Update to 5.0.4 also breaks some related managers resolution for models without inheritance, I can't clarify the reason as for now

@danjac
Copy link

danjac commented Sep 17, 2024

I found I had the same issue for plain Model classes, and the same problem with related managers just not working.

Ended up (in pyright) just setting reportAttributeAccessIssue to false as the number of error messages made attribute checking functionally useless (WithAnnotations also appears broken, but I think that's a different bug).

@vecchp
Copy link

vecchp commented Oct 8, 2024

We're also running into this issue as well if you bump to the latest stubs and mypy extension you will be able to reproduce the error.

https://github.com/BetterAngelsLA/monorepo/tree/main/apps/betterangels-backend

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

No branches or pull requests