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

Django stubs are out of date and cause erroneous type errors #6029

Open
simonwhitaker opened this issue Jun 18, 2024 · 7 comments
Open

Django stubs are out of date and cause erroneous type errors #6029

simonwhitaker opened this issue Jun 18, 2024 · 7 comments
Assignees
Labels
django Related to django support waiting for upstream Waiting for upstream to release a fix

Comments

@simonwhitaker
Copy link

simonwhitaker commented Jun 18, 2024

The Django stubs that ship with the Pylance VSCode extension are out of date, and take precedence over stubs installed in the local environment.

Screenshot 2024-06-18 at 5 06 14 PM

Environment data

  • Language Server version: 2024.6.1
  • OS and version: MacOS Sonoma 14.5
  • Python version (& distribution if applicable, e.g. Anaconda): 3.12.2

Code Snippet

n/a

Repro Steps

  1. In a Python 3.12 (virtual) environment:
    git clone git@github.com:simonwhitaker/pyright-stubs-demo.git
    cd pyright-stubs-demo
    pip install -r requirements.txt
  2. Open the cloned repo in VS Code
  3. Open manage.py
  4. Observe that RemovedInDjango51Warning on L6 is highlighted
  5. Observe the error: "RemovedInDjango51Warning" is unknown import symbol

Expected behavior

  • RemovedInDjango51Warning is a legitimate symbol in Django 5.0.6, and should not be highlighted. If you command-click (probably ctrl-click on Linux/Windows?) on django.utils.deprecation you will find it defined there.

Actual behavior

See screenshot above.

This appears to be because the django stubs shipped with the Pylance extension are out of date:

$ $ cat ~/.vscode/extensions/ms-python.vscode-pylance-2024.6.1/dist/bundled/stubs/django-stubs/utils/deprecation.pyi
from collections.abc import Callable
from typing import Any

from django.http.request import HttpRequest
from django.http.response import HttpResponse

class RemovedInDjango30Warning(PendingDeprecationWarning): ...
class RemovedInDjango31Warning(PendingDeprecationWarning): ...
class RemovedInDjango40Warning(PendingDeprecationWarning): ...
class RemovedInNextVersionWarning(DeprecationWarning): ...

class warn_about_renamed_method:
    class_name: str = ...
    old_method_name: str = ...
    new_method_name: str = ...
    deprecation_warning: type[DeprecationWarning] = ...
    def __init__(
        self,
        class_name: str,
        old_method_name: str,
        new_method_name: str,
        deprecation_warning: type[DeprecationWarning],
    ) -> None: ...
    def __call__(self, f: Callable[..., Any]) -> Callable[..., Any]: ...

class RenameMethodsBase(type):
    renamed_methods: Any = ...
    def __new__(cls, name: Any, bases: Any, attrs: Any) -> Any: ...

class DeprecationInstanceCheck(type):
    alternative: str
    deprecation_warning: type[Warning]
    def __instancecheck__(self, instance: Any) -> Any: ...

GetResponseCallable = Callable[[HttpRequest], HttpResponse]

class MiddlewareMixin:
    get_response: GetResponseCallable | None = ...
    def __init__(self, get_response: GetResponseCallable | None = ...) -> None: ...
    def __call__(self, request: HttpRequest) -> HttpResponse: ...

Logs

XXX
@github-actions github-actions bot added the needs repro Issue has not been reproduced yet label Jun 18, 2024
@rchiodo
Copy link
Contributor

rchiodo commented Jun 18, 2024

Thanks for the issue. We actually pull django stubs from here:
https://github.com/sbdchd/django-types

That's because the other django stubs (https://github.com/typeddjango/django-stubs) are mypy only and don't work with other type checkers.

It's likely the types you're talking about need to be added to https://github.com/sbdchd/django-types

@rchiodo rchiodo added waiting for upstream Waiting for upstream to release a fix django Related to django support and removed needs repro Issue has not been reproduced yet labels Jun 18, 2024
@simonwhitaker
Copy link
Author

Thanks for the issue. We actually pull django stubs from here: https://github.com/sbdchd/django-types

That's because the other django stubs (https://github.com/typeddjango/django-stubs) are mypy only and don't work with other type checkers.

It's likely the types you're talking about need to be added to https://github.com/sbdchd/django-types

Good to know! Thanks, I'll chase on that repo.

I do question however whether bundling a specific version of Django types in the extension is a good idea. My proposed "fix" will help me (working on Django 5.0), but will presumably cause problems for anyone working on an older version of Django. It would be great if the extension either:

a) didn't bundle a specific version of the django-stubs library at all, or;
b) gave an option to disable the bundled version somehow

What do you think?

@rchiodo
Copy link
Contributor

rchiodo commented Jun 19, 2024

a) didn't bundle a specific version of the django-stubs library at all, or;

What do you think?

I'm not sure if you meant we ship no type stubs or not, but the reason we bundle stubs is so we have type information for things which are not typed. Without type stubs you'd get very different intellisense from Django.

Here's an example:

def view(request, question_id):
    question = get_object_or_404(Question, pk=question_id)
    question. # Completions here

In that code above, if we remove type stubs, the type of question is unknown. So intellisense returns no members.

That's because the actual function definition isn't typed at all. We can't actually figure out the type that might be returned:

def get_object_or_404(klass, *args, **kwargs):

Whereas the type stubs provide types:

def get_object_or_404(
    klass: type[_T] | Manager[_T] | QuerySet[_T], *args: Any, **kwargs: Any
) -> _T: ...

If those type stubs aren't installed, Django is a lot harder to write (well in my opinion). That's why we ship them for some very common packages.

We have to pick a version to ship, and sometimes that version is out of date. Django is a tough one because the type stubs we use are not the most commonly maintained version. We have some ideas on how to address that, but it's a bit far down our backlog.

The best option for most people to get actual intellisense is to ship the latest type stubs. Most public APIs don't change all that often and when they do they're mostly additive (meaning newer type stubs work for the most part with older installs)

b) gave an option to disable the bundled version somehow

What do you think?

This might be the right idea for some people (people using older versions I'd think). Right now you can do this yourself by deleting our shipping stubs. That doesn't work so well though because you have to keep deleting them every time you upgrade.

So maybe an option to disable type stubs?

@simonwhitaker
Copy link
Author

If those type stubs aren't installed, Django is a lot harder to write (well in my opinion). That's why we ship them for some very common packages.

Honestly, having type stubs that are a couple of major versions out of date bundled with the extension makes Django pretty hard to write, too.

This might be the right idea for some people (people using older versions I'd think). Right now you can do this yourself by deleting our shipping stubs. That doesn't work so well though because you have to keep deleting them every time you upgrade.

Yeah, that's the problem. This just bit me again. My Pylance extension obviously updated itself, and I spent half an hour trying to remember how I fixed the erroneous type errors in my Django code the last time I saw them.

An option to disable type stubs would be really useful please.

@simonwhitaker
Copy link
Author

Incidentally, I did chase this on the source repo (sbdchd/django-types#255), but the issue hasn't been acknowledged.

@simonwhitaker
Copy link
Author

@rchiodo Any update on this one? It keeps biting our development team. A way to disable built-in type stubs would be very helpful here.

@rchiodo
Copy link
Contributor

rchiodo commented Jul 15, 2024

Sorry it's not currently planned. It's on our backlog but will likely take some time to get to. I'd recommend just submitting the new types to the https://github.com/sbdchd/django-types/blob/main/django-stubs/utils/deprecation.pyi as a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
django Related to django support waiting for upstream Waiting for upstream to release a fix
Projects
None yet
Development

No branches or pull requests

2 participants