django-stubs icon indicating copy to clipboard operation
django-stubs copied to clipboard

RelatedManager import moved breaking Pylance

Open martinlehoux opened this issue 1 year ago • 12 comments

Bug report

What's wrong

  • from django_stubs_ext.db.models import manager and usingrelated: manager.RelatedManager[OtherModel] breaks Pylance type hints in VSCode
  • from django.db.models import manager and usingrelated: manager.RelatedManager[OtherModel] breaks Mypy, but only in the same file

How is that should be

  • I know that there were improvements on RelatedManager automatic typing
  • We have always used Mypy as the source of truth for our types, so I guess we will use the new manager type, but this breaks VSCode completion, so it' a bit sad
  • Is there a way to have from django_stubs_ext.db.models import manager correctly export Django Manager so autocomplete works?

System information

  • OS:
  • python version: 3.12.0
  • django version: 4.2.8
  • mypy version: 1.7.1
  • django-stubs version: 4.2.7
  • django-stubs-ext version: 4.2.7

martinlehoux avatar Dec 11 '23 15:12 martinlehoux

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.

intgr avatar Dec 11 '23 15:12 intgr

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

martinlehoux avatar Dec 11 '23 17:12 martinlehoux

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.

intgr avatar Dec 13 '23 16:12 intgr

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 avatar Jan 10 '24 15:01 martinlehoux

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

martinlehoux avatar Jan 10 '24 15:01 martinlehoux

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.

baranyildirim avatar Feb 17 '24 12:02 baranyildirim

I tried @baranyildirim but this make mypy crash

martinlehoux avatar Feb 19 '24 17:02 martinlehoux

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

baranyildirim avatar Feb 19 '24 17:02 baranyildirim

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

akshetpandey avatar May 10 '24 21:05 akshetpandey

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.

PedroPerpetua avatar Jun 04 '24 20:06 PedroPerpetua

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.

intgr avatar Jun 05 '24 15:06 intgr

Thanks @intgr !

I tried the fix, but:

  • I could'nt find any bundled folder on my machine
  • When following the manager symbol in VSCode, which I think is powered by Pylance, I go to the correct stub file

martinlehoux avatar Jun 06 '24 08:06 martinlehoux

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

baojd42 avatar Aug 08 '24 12:08 baojd42

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

tylerlaprade avatar Aug 08 '24 17:08 tylerlaprade

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:

  • #579
  • #1264

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

flaeppe avatar Aug 09 '24 06:08 flaeppe