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

RelatedManager import moved breaking Pylance #1868

Open
martinlehoux opened this issue Dec 11, 2023 · 12 comments
Open

RelatedManager import moved breaking Pylance #1868

martinlehoux opened this issue Dec 11, 2023 · 12 comments
Labels
bug Something isn't working pyright Related to pyright type checker regression Behavior has changed for worse with a release

Comments

@martinlehoux
Copy link
Contributor

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 martinlehoux added the bug Something isn't working label Dec 11, 2023
@intgr intgr added pyright Related to pyright type checker regression Behavior has changed for worse with a release labels Dec 11, 2023
@intgr
Copy link
Collaborator

intgr commented Dec 11, 2023

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
Copy link
Contributor Author

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

@intgr
Copy link
Collaborator

intgr commented Dec 13, 2023

Are you still experiencing this issue? Another user reported that RelatedManager from django_stubs_ext works for them in Pylance: #765 (comment)

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
Copy link
Contributor Author

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
Copy link
Contributor Author

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
Copy link

baranyildirim commented Feb 17, 2024

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
Copy link
Contributor Author

I tried @baranyildirim but this make mypy crash

@baranyildirim
Copy link

baranyildirim commented Feb 19, 2024

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
Copy link

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

@PedroPerpetua
Copy link

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
Copy link
Collaborator

intgr commented Jun 5, 2024

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

As for #1868 (comment)

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
Copy link
Contributor Author

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working pyright Related to pyright type checker regression Behavior has changed for worse with a release
Development

No branches or pull requests

5 participants