search

typeddjango / django-stubs

PEP-484 stubs for Django
MIT License
1.58k stars 436 forks source link

RelatedManager import moved breaking Pylance #1868

Closed martinlehoux closed 1 month ago

martinlehoux commented 9 months ago

Bug report

What's wrong

How is that should be

System information

intgr commented 9 months ago

from django_stubs_ext.db.models import manager and usingrelated: manager.RelatedManager[OtherModel] breaks Pylance type hints in VSCode

If we can figure out why pyright/pylance doesn't like django_stubs_ext.db.models.manager.RelatedManager then we might be able to fix it. This would be the preferred solution. Unfortunately I don't have much experience with pyright.

from django.db.models import manager and usingrelated: manager.RelatedManager[OtherModel] breaks Mypy, but only in the same file

Yep, that symbol no longer exists.

You may be able to resolve this by using from django.db.models.fields.related_descriptors import RelatedManager or similar, behind an if TYPE_CHECKING guard. But that's a work-around at best.

martinlehoux commented 9 months ago

Thanks! I'll try to have a look then on how Pylance works with this file

intgr commented 9 months ago

Are you still experiencing this issue? Another user reported that RelatedManager from django_stubs_ext works for them in Pylance: https://github.com/typeddjango/django-stubs/issues/765#issuecomment-1854251782

If yes, please double check that you have the right version of both django-stubs and django-stubs-ext in whatever environment that Pylance runs with.

If that doesn't help, post what errors you are getting.

martinlehoux commented 8 months ago

Hello! I am still experiencing the issue (it's not really an error)

The best I can explain is that when I try to look for the manager.RelatedManager definition, Pylance sends me to django_stubs_ext.db.models.manager.py

From there, Pylance seems to never evaluate the else statement, but doesn't seem to resolve "stubs-only" classes

image by As a side note, if I replace from django.db.models.fields.related_descriptors import RelatedManager as RelatedManager by from django.db.models.manager import RelatedManager, follow the symbol sends me to a vscode-pylance specific bundle that has the correct typing

martinlehoux commented 8 months ago

It is possible that it is my setup that is incorrect : I can see that some symbols resolve to a django .py source file, while others resolve to a .pyi django stubs file

baranyildirim commented 7 months ago

Also doesn't work for me in mypy. I think we need to invert the logic here:


if TYPE_CHECKING:
    # noinspection PyUnresolvedReferences
    from django.db.models.fields.related_descriptors import ManyRelatedManager as ManyRelatedManager

    # noinspection PyUnresolvedReferences
    from django.db.models.fields.related_descriptors import RelatedManager as RelatedManager

else:
    from typing import Protocol, TypeVar

    _T = TypeVar("_T")

    # Define as `Protocol` to prevent them being used with `isinstance()`.
    # These actually inherit from `BaseManager`.
    class RelatedManager(Protocol[_T]):
        pass

    class ManyRelatedManager(Protocol[_T]):
        pass

to 

if not TYPE_CHECKING:
    from typing import Protocol, TypeVar

    _T = TypeVar("_T")

    # Define as `Protocol` to prevent them being used with `isinstance()`.
    # These actually inherit from `BaseManager`.
    class RelatedManager(Protocol[_T]):
        pass

    class ManyRelatedManager(Protocol[_T]):
        pass

else:
    # noinspection PyUnresolvedReferences
    from django.db.models.fields.related_descriptors import ManyRelatedManager as ManyRelatedManager

    # noinspection PyUnresolvedReferences
    from django.db.models.fields.related_descriptors import RelatedManager as RelatedManager

This change has fixed the issue on my end.

martinlehoux commented 7 months ago

I tried @baranyildirim but this make mypy crash

baranyildirim commented 7 months ago

I tried @baranyildirim but this make mypy crash

Ignore my previous comment - from django_stubs_ext.db.models.manager import RelatedManager is what I needed.

I have a configuration that uses both pylance and mypy. Pylance definitions don't work with this solution.

RelatedManager is only defined inside a function (create_reverse_many_to_one_manager) in django.db.models.fields.related_descriptors so how is this import supposed to work:

from django.db.models.fields.related_descriptors import RelatedManager as RelatedManager
akshetpandey commented 4 months ago

This works as in, it doesn't cause pylance to crash... but pylance resolves it to ANY

PedroPerpetua commented 4 months ago

I just encountered this problem (importing from django_stubs_ext works but pylance resolves it to Any) and re-installing the extension (Pylance) fixed it for me.

intgr commented 4 months ago

I think the issue might be https://github.com/microsoft/pylance-release/issues/5031, where Pylance's bundled stubs take precedence over manually installed django-stubs. If someone would try the work-around suggested at https://github.com/microsoft/pylance-release/issues/5031#issuecomment-1888387625 and report back, we could confirm or reject this hypothesis.

As for https://github.com/typeddjango/django-stubs/issues/1868#issuecomment-1949991939

I think we need to invert the logic here

That is not correct. In TYPE_CHECKING=True case, these need to be imported from django-stubs for the real definitions. Otherwise they would be just empty protocols.

martinlehoux commented 4 months ago

Thanks @intgr !

I tried the fix, but:

baojd42 commented 1 month ago

If someone would try the work-around suggested at microsoft/pylance-release#5031 (comment) and report back, we could confirm or reject this hypothesis.

I've tried the workaround. Pylance then correctly picks installed django-stubs instead of the bundled django-types, and RelatedManager can be imported from django_stubs_ext.db.models.manager.

However, after being switched to django-stubs, Pylance lost the inference for the fields' types. I have to explicitly write the type hints for ForeignKey like author = models.ForeignKey[Author, Author](Author, on_delete=models.CASCADE), otherwise author will be inferred to Unknown (Pylance with type checking mode set to basic). Even for PositiveSmallIntegerField, I have to write field_a = models.PositiveSmallIntegerField[int, int], or the type field_a will also be inferred to Unknown. If I'll have to write type hints for every field, it will be too tedious.

@intgr

tylerlaprade commented 1 month ago

I agree. django-types is a non-starter compared to django-types for projects using Pyright/Pylance.

flaeppe commented 1 month ago

I've tried the workaround. Pylance then correctly picks installed django-stubs instead of the bundled django-types, and RelatedManager can be imported from django_stubs_ext.db.models.manager.

However, after being switched to django-stubs, Pylance lost the inference for the fields' types. I have to explicitly write the type hints for ForeignKey like author = models.ForeignKey[Author, Author](Author, on_delete=models.CASCADE), otherwise author will be inferred to Unknown (Pylance with type checking mode set to basic). Even for PositiveSmallIntegerField, I have to write field_a = models.PositiveSmallIntegerField[int, int], or the type field_a will also be inferred to Unknown. If I'll have to write type hints for every field, it will be too tedious.

@intgr

Thank you for confirming that the suggested workaround solved the reported problem. I will close this issue as completed, I think the other problems you mention are tracked/covered by these issues:

Feel free to open any new issue/discussion if you feel that you encounter something that hasn't been reported.