@mkurnikov please review this PR from your side as well! 👍

Hi. Thanks for opening this. I'm going to ask the Technical Board to make a decision on this topic.

I just want to point out that previous version of this patch adds __class_getitem__() only to two classes (QuerySet and Manager). I'm not sure why the current version adds it to 4 classes (Field, Lookup, BaseManager, and QuerySet), and I also don't know how we want to decide where we need it in the future, and how we want to prevent regressions in this area without any tests (that are not feasible, as far as I'm concerned) 🤔 In summary, I'm -1 👎 .

@felixxm Would you like to see tests like assert QuerySet[] is QuerySet? Could you clarify if you are talking about regressions at runtime or in static type checks?

and I also don't know how we want to decide where we need it in the future

Is that an argument against this implementation? To me that's a very strange argument, and it sounds like "due to not knowing what will happen in the future, we should do nothing today". In the discussion on the DEP there seemed to be consensus that this is an insignificant change that will unlock very valuable features for users of static typing in Django. I think it would be very sad if Django entirely turns its back on static typing.

I'm not sure why the current version adds it to 4 classes

This is a valid concern, although @mkurnikov did mention "maybe also other classes" in his proposal in the DEP discussion. Maybe @sobolevn and/or @mkurnikov could shed some light on which classes critically needs subscriptability, would it be a good idea to add docstrings to the __class_getitem__ methods detailing what TypeVars they are expected to accept? Or what they are variable in regards to. (E.g. in the form QuerySet is expected to vary on a TypeVar with an upper bound of Model).

That's how I made a decision about which classes need to have __class_getitem__: inside django-stubs we define type stubs inside .pyi and some classes need to be Generic[...].
I have found which classes we mark as Generic inside our implementation and added __class_getitem__ to them here.

So users can write QuerySet[MyModel] instead of 'QuerySet[MyModel]', because the second one is confusing and ugly. Docs: https://github.com/typeddjango/django-stubs#notes

I have omitted cached_property, because it does not make much sense outside of typing. And I want to keep this PR as minimal as possible.

Searching for Generic in django-stubs: https://github.com/typeddjango/django-stubs/search?l=Python&p=2&q=Generic

1. Manager and QuerySet are critical, it's not about QuerySet['MyModel'] vs Queryset[MyModel], it's more to allow use things like

class MyQuerySet(QuerySet['MyModel']):
    pass

class MyManager(Manager['MyModel']):
    pass

class MyOtherModel(models.Model):
    objects = Manager['MyOtherModel']()
    objects2 = MyManager['MyOtherModel'].from_queryset(MyQuerySet)

2. Field generic needed for field subclassing

class MyCharField(CharField[_ST, _GT]):
    pass

Generics on fields used for __get__ and __set__, and should be present to be able to modify return type of __get__ in presence of null=True, for example. But it's more or less implementation detail, there's just no other way to change __get__ return type without storing generic type on type instance.
(We could reimplement whole descriptor machinery in the plugin, I guess).

If there's an opposition to this change, one could always just give up on __get__, __set__ return types on custom fields, so non-essential.

3. Generic parameter on Lookup in stubs is supposed to be a marker of possible argument type.

class IsNull(BuiltinLookup[bool]): ...
class Regex(BuiltinLookup[str]): ...

It's plugin implementation detail of sorts, and could be reimplemented into hard-coding of lookups with their corresponding value types

{
    IsNull: bool,
    Regex: str
}

We could also go with Field style of private properties

class CharField(Field[_ST, _GT]):
    _pyi_private_set_type: Union[str, int, Combinable]
    _pyi_private_get_type: str

It's cleaner to have it right in stubs, and allow to smoothly create new Lookup types, but really non-essential. Could be dropped from PR.

Oh, and there's a PR to CPython open right now, python/cpython#18239, which adds a number of __class_getitem__ implementations to the standard library. It could be used as inspiration for tests.

1. Manager and QuerySet are critical, it's not about QuerySet['MyModel'] vs Queryset[MyModel]

Agreed. +1.

2. Field generic needed for field subclassing

class MyCharField(CharField[_ST, _GT]):
    pass

The API of CharField[_ST, _GT], with 2 parameters gives me pause. I'd recommend tackling that in a separate PR. -1.

3. Generic parameter on Lookup in stubs is supposed to be a marker of possible argument type.

class IsNull(BuiltinLookup[bool]): ...
class Regex(BuiltinLookup[str]): ...

I understand the syntax. Doesn't feel critical for this first round. -0.

@davidfstr Just to be clear: this PR only introduces __class_getitem__ method to be used later inside django-stubs, our typing API is our implementation detail. It should not be taken into consideration right now.

Hi all. I write up something for the technical board next week now. Can I ask you to agree the minimal form of this PR (and adjust the actual changes if needed) so that it's canonical when I ask them to look?

@davidfstr

The API of CharField[_ST, _GT], with 2 parameters

__set__ type of CharField is Union[str, int, Combinable], and __get__ is just str.

@sobolevn
Let's remove controversial __class_getitem__ on Field and Lookup for now to ease review and acceptance. We could work on it later, no rush for those.

@mkurnikov sure thing!

@sobolevn
Could you also add a test somewhere with assert QuerySet[] is QuerySet and some other combinations? For Manager too. Just to prevent regressions on this front, and add some test coverage. Thanks.

Tests are very hard to navigate, so I have ended up adding two simple cases to places where inheritance is checked.

@sobolevn Can you check new tests locally before pushing? (gentle request)

No, sorry. My current device does not allow that. I can run tests locally in more than a week. I use Github web editor right now.

Just curious -- is there anything holding this up?

I also eagerly wish for this fix! It will open the door to the world without no ugly ”# type: ignore” in my codebase. 😅

It's under consideration by the Technical Board. The earliest it would possibly be released is for Django 3.1, so there is no urgency.

There is no "urgency" but the sooner Django project's position is stated, the sooner we can begin discussing additional type annotations that might reach Django 3.1.

OK, thank you for your patience everybody.

Statement from the Technical Board is here:
https://groups.google.com/d/topic/django-developers/C_Phs05kL1Q/discussion

We will be moving forward with this PR for Django 3.1.