Open kylebebak opened 3 years ago
Does pyright
support descriptors? It's what drives model field support.
Hey @mkurnikov ,
pyright
supports descriptors, and features from just about all the other PEPs related to type annotations!
There's a fork of this library maintained by @sbdchd, which makes some changes so the stubs work with pyright
, and with mypy
without the plugin. Getting the types to work for models requires some manual type annotations, but it's not a lot of work.
Here's an example:
from __future__ import annotations
# ...
class Thing(models.Model):
objects: Manager[Thing]
related_things: RelatedManager[RelatedThing]
other_thing: models.ForeignKey[OtherThing]
other_thing_id: UUID
I added types to all the models in a fairly large code base in less than a day, and the code base is now a lot easier to work with and navigate.
The maintainer of the fork told me there was a decent amount of work to get the annotations to work without the plugin. I wonder whether there's a way for this repo to ship types that work with and without the plugin, so users of other type checkers can use these annotations. They're very powerful.
I don't see any reason why the django-stubs
couldn't also use the approach by the fork.
For example, of using overloads with Literal types to handle nullable fields: https://github.com/sbdchd/django-types/commit/64a5b3b118fe07d9b3eba16af4c18ecbd5bb8f1e#diff-876a5d116ef70c86039fbb8498623871324fa93fc1ae51c7c09ba2a0f7612e99R187
I doubt it would require a lot of changes to the plugin. I think the main reason this wasn't done earlier was because using overloads on __init__
with narrowing of the self
type wasn't supported by any type-checkers at the time the stubs were written.
I am not sure about some of the other changes made by in the fork, but they are also likely possible to integrate into django-stubs
.
It would be nice if the maintainers of the 2 projects could talk together about an approach to getting both mypy (with or without the plugin) + pyright to work on the same set of stubs, rather than having 2 separate versions.
Yes, making django-stubs
available for other type-checkers and minimizing plugin part is 100% a goal of this project.
Hey guys!
What is the progress of this issue? Like other have said here, the work that @sbdchd has done on https://github.com/sbdchd/django-types is amazing, and the way dealt with some overloads is pretty clever. I can't see why django-stubs couldn't do the same (probably merge his changes?), and only do mypy magic where it needs to be done.
Basically, based on my experience using his fork and trying to type everything in all projects that I have, here are the problems that can't be solved by the typing/stubs lib itself, and some thoughts regarding each:
1) No automatic resolution to Model.objects
:
Before this would require manual typing or mypy magic, but this can be solved by creating this inside ModelBase
(Model's metaclass):
_M = TypeVar("_M", bound=Any)
class ModelBase(type):
@property
def objects(cls: Type[_M]) -> BaseManager[_M]: ...
So, something like this can be incorporated without having to rely on mypy magic.
2) No default id
field in the model:
This one requires mypy magic. Without it, anyone using something else needs to type the id manually (if it is used)
3) ForeignKey ids:
The same as the previous one, it requires mypy magic. Without it, anyone using something else needs to type the id manually: E.g.
class User(models.Model):
team_id: Optional[int]
team = models.ForeignKey(
Team,
null=True,
on_delete=models.SET_NULL,
)
role_id: int
role = models.ForeignKey["Role"](
"Role",
null=False,
on_delete=models.SET_NULL,
related_name="users",
)
4) Related fields:
Same as the previous. This one can be workarounded by:
class Team(models.Model):
if TYPE_CHECKING:
user_set = RelatedManager["User"]()
class Role(models.Model):
if TYPE_CHECKING:
users = RelatedManager["User"]()
5) Validate values in Model.__init__
and Model.objecrs.create
:
This one doesn't have really a way to be fixed, but, maybe this can be used to help we get around it: https://github.com/microsoft/pyright/blob/main/specs/dataclass_transforms.md
6) Validate lookups in Manager.filter
.
This one really needs mypy magic. Can't think of any kind of workaround for now.
============
So, other than the 6 cases above (or anything more that I forgot to mention), everything else can be solved by the current typing specifications without the need of the mypy plugin.
Also, number 1 can probably be fixed too like I mentioned and maybe 5 depending on how dataclass_transforms works. Everything else is something that the mypy plugin will still be needed.
@sobolevn @mkurnikov @syastrov
Any reason we can't do what @bellini666 is suggesting and merge most of the changes that have been made in https://github.com/sbdchd/django-types into this repo?
Making these type hints usable with type checkers other than mypy
, e.g. pyright
, would make this excellent library useful to a lot more people.
I would love to merge it 🙂
If anyone can send a PR, this would be amazing.
@bellini666 Thanks for putting together this list. It's interesting to see the areas in Django which are not currently covered by Python's static type system listed in one place.
- ForeignKey ids:
The same as the previous one, it requires mypy magic. Without it, anyone using something else needs to type the id manually: E.g.
class User(models.Model): team_id: Optional[int] team = models.ForeignKey( Team, null=True, on_delete=models.SET_NULL, ) role_id: int role = models.ForeignKey["Role"]( "Role", null=False, on_delete=models.SET_NULL, related_name="users", )
I wonder if Django could be taught that you can either pass int
or a Role
instance to user.role
here (and the same logic could arguably apply to Role.__init__
, which could compose well with the dataclass transforms
PEP if that were to support Django's use case).
Also, when accessing user.role.id
, instead of fetching the entire Role
object when it wasn't previously fetched (e.g. by select_related
), a "lazy" Role
object could be constructed which only has the id
field populated -- to avoid an extra query to the database.
This would not help for existing code, but for new code, it would help avoid this manual work in getting static typing to work. The old way could be deprecated if people agree that this is a better way forward.
That said, I'm not sure how amenable the Django project is to these kinds of changes.
An alternative would be to propose some sort of PEP that allows a descriptor defined on a class to add arbitrary extra attributes to a class, whose name is based on its name and whose type is somehow related to its own type. That second part could get really nasty though: you'd have to get it to extract the type of the primary key of the referenced model. This entirely seems too specific to Django, so I don't see it getting much acceptance.
- Related fields:
Same as the previous. This one can be workarounded by:
class Team(models.Model): if TYPE_CHECKING: user_set = RelatedManager["User"]() class Role(models.Model): if TYPE_CHECKING: users = RelatedManager["User"]()
It's a bit nasty that it requires an if TYPE_CHECKING
, but arguably, it is nice to be explicit here. I often find myself forgetting that Django autogenerates the related managers when related_name
is not specified, and people end up exposing the "ugly" default name (e.g. team.user_set
) rather than a custom one (e.g. team.members
).
Maybe Django could support something like this in the future (not sure if it's possible)?
class Team(models.Model):
members = RelatedManager(User.team) # This registers the related_name for the User.team foreign key
class User(models.Model):
# related_name is not necessary here.
# If specified, it should give a runtime error, since there is already a related_name registered above.
team = ForeignKey(Team)
- Validate values in
Model.__init__
andModel.objecrs.create
:This one doesn't have really a way to be fixed, but, maybe this can be used to help we get around it: https://github.com/microsoft/pyright/blob/main/specs/dataclass_transforms.md
I am also keeping an eye on that PEP. Unfortunately, it seems like it would only apply to Model.__init__
.
- Validate lookups in
Manager.filter
.This one really needs mypy magic. Can't think of any kind of workaround for now.
The mypy magic is fun for supporting the current state of things, but maybe Django (or a third-party package) could offer an API similar to SQLAlchemy where a reference to the field's definition is explicitly mentioned -- something like Model.objects.filter(Model.int_field > 3)
(e.g. field descriptors such as Model.int_field
, would have dunder methods such as __gt__
implemented on them as classmethod
s such that when called would produce the corresponding Q
object which could be passed to filter
and friend). It could also be spelled as Model.objects.filter(Model.int_field.gt(3))
), which is less magical and could correspond more exactly to the names of the "lookup expressions".
I am doubtful that Python's static type system would ever come to support the current Django-way of lookups expressions (beyond having to write a million overloads), so this is why I think proposing an alternative spelling would be nice. I am not sure if Django itself would be receptive to this, but it could at least be implemented in a third-party package through monkey-patching for those who are interested in static typing of conditions.
Manager.from_queryset
To be typed properly without boilerplate and without mypy magic, it would probably require intersection types to be implemented in Python's type system (?).
There are probably other minor cases, but I think you've identified the important ones.
I see several possible ways of meeting these problems without a plugin which could be taken on a case-by-case basis:
I think it could be really interesting to try out (4), since it essentially would allow supporting a large subset of the magic, even in other type-checkers than mypy.
I have a simple question - how are you all importing RelatedManager
such that the following code works?
if TYPE_CHECKING:
user_set = RelatedManager["User"]()
@craiglabenz you can either monkey patch RelatedManager so that you can pass the ["User"]
argument at runtime, or import RelatedManager
behind a if TYPE_CHECKING
statement like:
from typing import TYPE_CHECKING
if TYPE_CHECKING:
# This doesn't really exists on django so it always need to be imported this way
from django.db.models.manager import RelatedManager
from user.models import User
class Team(models.Model):
if TYPE_CHECKING:
user_set = RelatedManager["User"]()
or with monkey patching to avoid the if TYPE_CHECKING:
# in settings.py
from django.db.models import ForeignKey
from django.db.models.manager import BaseManager
from django.db.models.query import QuerySet
# NOTE: there are probably other items you'll need to monkey patch depending on
# your version.
for cls in [QuerySet, BaseManager, ForeignKey]:
cls.__class_getitem__ = classmethod(lambda cls, *args, **kwargs: cls) # type: ignore [attr-defined]
Just to update this, django-stubs version 4.2.6 no longer has a dependency on mypy and we're trying to facilitate small pull requests to improve experience with other type checkers.
Does anyone have an example of a working pyproject.toml
that sets up [tool.pyright]
to work with django-stubs
?
(I'm trying to get pylance/pyright working with django-stubs
in VSCode)
@pirate I still had to ignore reportAttributeAccessIssue
, but:
[tool.pyright]
# https://github.com/microsoft/pyright/blob/main/docs/configuration.md
exclude = [
# ...
]
# Pylint rule list:
# https://pylint.readthedocs.io/en/stable/user_guide/checkers/features.html
lint.ignore = [
# "E501", # line too long
"R0903", # too few public methods
]
pythonPlatform = "Linux"
# "none", "warning", "information", or "error"
reportMissingTypeArgument = "information"
reportPrivateUsage = "information"
typeCheckingMode = "standard" # "off", "basic", "standard", "strict"
# Reports:
# https://github.com/microsoft/pyright/blob/main/docs/configuration.md#type-check-diagnostics-settings
# place ignored rules here
reportAttributeAccessIssue = false # too many false positives with Django models
Apparently, the large amount of errors that appear in the CI pyright step is blocking its adoption in Pylance (part of the VS Code Python extension, very popular).
Source: https://github.com/microsoft/pylance-release/issues/6029#issuecomment-2276671070
Hey all,
I know the README says it doesn't make sense to use
django-stubs
without mypy, because mypy uses a plugin for cases where type hints alone don't work. Sorry if this question is a waste of time, and feel free to close this issue.That said, this library ships with some very thorough type stubs for just about everything in Django. Is it possible to get a subset of the benefits from these stubs using a different type checker, like
pyright
?I tried using the stubs from the latest commit, without the mypy plugin, with pyright, but have found that
pyright
can't infer the types of model fields, which would be very useful. Nor can it infer types returned byManager
methods.I'm sure pyright is sourcing the stubs just fine; I can manually edit the stubs for something like
timezone.now()
, and pyright picks it up immediately.@sobolevn @mkurnikov
Is it expected that most things having to do with the ORM don't work without running the mypy plugin? Is there manual monkey-patching that can be done, without running the mypy plugin, to help type checkers infer the correct types from model fields and Manager methods?
If this were possible it would be really awesome, and would help more users of type hints get lots of value out of
django-stubs
=)I'm using Django 3.1.6, and stubs from the latest commit.
Examples
Here are some images of
pyright
doing type inference in my text editor:I instantiate a
prod
from aProduct
models, which has aname
field that's aCharField
.pyright
knowsprod
is aProduct
, but it thinksprod.name
has a type ofUnknown
. It thinksProduct.name
has a type ofCharField[Unknown, Unknown]