sbdchd / django-types

:doughnut: Type stubs for Django
MIT License
187 stars 62 forks source link

Options for cooperation with django-stubs #207

Open intgr opened 10 months ago

intgr commented 10 months ago

Hi! I'm one of the maintainers of the django-stubs project.

I sense that there is significant duplication of effort between django-stubs and django-types. Our pace of development has picked up recently, but still we're struggling to cover the Django API surface. If we joined forces, we could get more done.

If you're open to that, we need an understanding about the differences between the projects, here's what I'm aware of:

  1. I think the initial reason for forking was that django-stubs requried mypy? As of django-stubs version 4.2.6, mypy is no longer a required dependency.
  2. django-stubs relies on our mypy plugin magic for inferring Models/QuerySets. django-types instead documents how users should annotate Django's magic attributes on Model classes.
  3. There are differences in how mypy and pyright interpret type stubs. Hopefully not mutually incompatible, other projects (e.g. typeshed) work fine with multiple type checkers. We have already seen some contributions from pyright users.
  4. Discussion #147 points out that django-stubs uses _pyi_private_... attributes in Model fields for our magic. I wouldn't think their existence interferes when using pyright though?

(1) is solved and the rest don't look insurmountable.

Some relevant discussion:

Ping @sobolevn for visibility.

intgr commented 10 months ago

I found this in Django wiki (GSoC 2023): https://code.djangoproject.com/wiki/SummerOfCode2023#Mergingdjango-typesbackintodjango-stubs

I'm wondering if you agree with this characterisation:

There is a spin-off project from django-stubs called django-types. The original intention here was for it to be merged back into django-stubs, but that's never happened.

django-types removes the need to use any mypy plugins, making it possible to use type checkers other than mypy, such as Pyrite, Pyre, etc.

django-types is not actively maintained, however, and having competing stubs implementations means neither is as strong as could be.

There is a small project in extracting the work from django-types and creating a PR for django-stubs allowing it to be merged back in, and effort to be concentrated on the single implementation.

intgr commented 10 months ago

Here's also a description about django-stubs/django-types differences:

sbdchd commented 10 months ago

I found this in Django wiki (GSoC 2023): https://code.djangoproject.com/wiki/SummerOfCode2023#Mergingdjango-typesbackintodjango-stubs

I'm wondering if you agree with this characterisation:

There is a spin-off project from django-stubs called django-types. The original intention here was for it to be merged back into django-stubs, but that's never happened. django-types removes the need to use any mypy plugins, making it possible to use type checkers other than mypy, such as Pyrite, Pyre, etc. django-types is not actively maintained, however, and having competing stubs implementations means neither is as strong as could be. There is a small project in extracting the work from django-types and creating a PR for django-stubs allowing it to be merged back in, and effort to be concentrated on the single implementation.

Tbh it was a while ago but I think the original intention was to get autocomplete to work in VSCode to make my job (and coworkers) working in the Django codebase easier. I was cool with maintaining the project solo since we didn’t use that many Django features, a small surface area. I think we only used Django as a JSON API so the admin and related tools weren’t that important.

So once the models and query sets were working I considered the project good enough and would occasionally add some more types if autocomplete wasn’t working in some cases.

Ideally I was hoping for Django to incorporate types but I think they said no a while back.

I no longer work for that company or work in Django regularly so this project is more of a, “whenever I check GitHub and have the time to merge a PR”

I’m open to merging the projects, I think there would need to be parity since people are using this project as a dependency

Viicos commented 9 months ago

One key difference between the two libraries can be found in related fields: django-types is similar to django-stubs for RelatedField:

But starting at ForeignObject and subclasses, signature differs:

With django-types annotating a ForeignKey for example is pretty straightforward:

class Model(models.Model):

    article = models.ForeignKey["Article"]("Article", on_delete=models.CASCADE)

Model().article  # revealed type is Article

but this is unfortunately not possible with django-stubs, as you need to parametrize both the set and get type:

class Model(models.Model):

    article = models.ForeignKey["Article", "Article"]("Article", on_delete=models.CASCADE)

Model().article  # revealed type is Article, revealed type is Any if using the first code snippet

Is there any reason for django-stubs to have both a distinct set and get type for forein keys & co?

Edit: forgot that you could set Model().article = <a_pk>, hence the need for the two type vars :/

mordae commented 9 months ago

Edit: forgot that you could set Model().article = <a_pk>, hence the need for the two type vars :/

Which can be practically worked around by setting article_id instead.

tylerlaprade commented 9 months ago

FWIW, when I use django-types, I don't even need to annotate the type of ForeignKey. Pyright just knows the type out of the box. In django-stubs, it ends up unknown.

Viicos commented 9 months ago

FWIW, when I use django-types, I don't even need to annotate the type of ForeignKey. Pyright just knows the type out of the box. In django-stubs, it ends up unknown.

Could you be more specific? Is it unknown when accessing the foreign key field from an instance? From the class? By unkown, do you mean the actual Unknown type or Any? Are you using a string reference to the model, or a direct reference to the model?

tylerlaprade commented 9 months ago

That's right, it's Unknown in Pyright when I try to directly reference it.

image image

The same code when not using django-stubs and only using django-types:

image image

This means that with django-stubs, I only get type-safety for the first level of accessing attributes - if I did fx_rate.foo, that would be an error. But I don't get type-safety beyond that - I can do fx_rate.company.foo without any errors, while django-types catches that.

sobolevn commented 9 months ago

That's exactly the point of this issue: to make django-stubs and pyright compatible.

intgr commented 9 months ago

Yep. I have looked into the tricks that django-types uses to get better type inference for model fields without the plugin. It should be possible to achieve the same results one way or another, without degrading experience with mypy. But I haven't had the time to continue with that work.

tylerlaprade commented 9 months ago

@sobolevn, I was clarifying since @Viicos's message implied that one would need to manually annotate even with django-types, which is not the case (at least in Pyright, I'm not sure about Mypy).

Viicos commented 9 months ago

@tylerlaprade Thanks for the clarification. Both mypy and pyright should support the provided snippet with django-types. But this only works when using a direct reference to the model (instead of the string version {app_label}.{model_name}/{model_name}).

Actually django-stubs could support this use case as well by defining:

class ForeignKey(ForeignObject[_ST, _GT]):
    # As an extra overload:
    def __init__(
        self,
        to: type[_GT],
        ...
    )

But the set type can't really be parametrized this way, and this doesn't take into account nullability anyway.

FYI, I'm working on an alternative solution to support dango-stubs natively without plugins and without modifications to the actual type variables used. I will share more info soon.

jereze commented 9 months ago

I apologize for appearing in the middle of the discussion, but this might be related. I was recently searching for how to get type hinting for ForeignKey fields using string references in Vscode (which uses django-types), and I got some advice in this discussion.

Viicos commented 9 months ago

Is there any reason for django-stubs to have both a distinct set and get type for forein keys & co?

Edit: forgot that you could set Model().article = , hence the need for the two type vars :/

After further investigation and looking at the descriptor implementation, you can't set the foreign field by it's primary key, so I don't think having two type vars for foreign fields is necessary? Or at least make both the set and get type the same typevar.

Or am I missing something?

Edit: It seems to be related with Combinable, which can be used on the set type. This seems to be the reason?

Viicos commented 9 months ago

FYI, I'm working on an alternative solution to support dango-stubs natively without plugins and without modifications to the actual type variables used. I will share more info soon.

You can find more info about this in the following article: https://viicos.github.io/posts/an-alternative-to-the-django-mypy-plugin/

django-autotyping is available here: https://github.com/Viicos/django-autotyping

Would love getting some feedback on this.

Yep. I have looked into the tricks that django-types uses to get better type inference for model fields without the plugin. It should be possible to achieve the same results one way or another, without degrading experience with mypy. But I haven't had the time to continue with that work.

I've been looking at this when implementing django-autotyping. I will come back here later with my findings on this subject.

tylerlaprade commented 9 months ago

@Viicos, interesting idea, but I don't understand why it's necessary. Pyright already knows the underlying type of a ForeignKey without any annotations when using django-types.

Viicos commented 9 months ago

I don't understand why it's necessary

Well as I said here, it doesn't support string references to models, which are pretty common (I would even recommend using them over direct reference to the actual related class for different reasons). The article also explains a bit about this.

Apart from support for ForeignKey, the tool will support a lot of features from the mypy plugin:

tylerlaprade commented 9 months ago

Ah I see, thanks for clarifying!