Replace TypeAlias _ClassLevelWidgetT with descriptor _WidgetTypeOrInstance

[michalpokusa]

I have made things!

Related issues

None I could find.

This change allows for recognizing when .widget is used on Field instance and when on the type itself, allowing for different type annotations in each case.

Why even bother?
Example: Currently this correct code does not recognize is_required and attrs on field.widget, because it might be a type and not an instance.

After the change all types are correctly inferred, and syntax highlighting works as expected:

I hope you find it worth adding.

[sobolevn]

Looks like a good idea! Thanks!

Please, fix the CI :)

[michalpokusa]

@sobolevn I think I might need some help there. I checked why the CI is failing and it looks like errors are not even slightly related to what I have changed.

I checkouted the current origin/HEAD, ran the test locally and also I got a lot of errors. Is there something obvious I am missing? I believe there might be a problem with the CI itself as I see that on other PR #2616 also fails in the same way.

[UnknownPlatypus]

Setuptools version 79 got installed, I suppose it probably breaks the multiple SETUPTOOLS_ENABLE_FEATURES in ci

[sobolevn]

you can pin older setuptools for now in a separate PR :)

[sobolevn]

Can you please also add several type assertions for this? https://github.com/typeddjango/django-stubs/tree/master/tests/assert_type

[michalpokusa]

@sobolevn I started adding type assertions, and I would like to get you point of view on one thing. I could leave this as typing everything as the most general Widget, or I could make it generic, and type widget individually for each field type.

Example:

It provides more precise typing, but I am not sure whether it is not too restrictive.
On the other hand MultipleChoiceField could have a widget of type SelectMultiple or CheckboxSelectMultiple.
And how should it work with custom field types and custom widgets, wouldn't it break them?

What do you think about that?

[sobolevn]

Let's first add the simplest typing possible. We can later update it to cover more cases.

[michalpokusa]

I have added assert type tests.

Seems like this is a bit more complex than I initailly assumed, right now there is a problem with stubtest.
All the errors center around widgets being MediaDefiningClass instances. The interesting thing is that e.g. Field.hidden_widget is typed as type[Widget], while BaseFormSet.deletion_widget as MediaDefiningClass inside ".pyi" files.

Any further guidance would be highly appreciated.

[sobolevn]

Based on the amount of failures I would say that this seems overly complex to type properly :(

However, I am open to ideas.

[michalpokusa]

Would it be possible to supress these errors? It seems like they are a side affect of some problem with django/mypy itself.
I suppose it is related to the Widget(metaclass=MediaDefiningClass), but the Widget instance is not a MediaDefiningClass subclass.

I tried to fix the errors in .pyi files, but the errors seem to be simply inaccurate. It they could be supressed, users could benefit from the better typing while not technically lying about the typing to mypy, as it seems like it gets it wrong in this case.

Any opinion on that would be appreciated.

[UnknownPlatypus]

Would it be possible to supress these errors? It seems like they are a side affect of some problem with django/mypy itself.
I suppose it is related to the Widget(metaclass=MediaDefiningClass), but the Widget instance is not a MediaDefiningClass subclass.

I think it might be actual error for the types when working with a field class, not an instance of a field. I do agree the error message is a bit weird tho but the def (attrs=None) signature is Widget.__init__, which makes me think the type is incorrect for subclasses.

Could you add a test with a custom Field using a custom widget ? Something like that:

class MyField(Field):
    widget = Input

I think it will fail for the same reason as in #2650, I couldn't find a way to make it work with this approach for a similar need, maybe you'll find a way. Basically the approach with a descriptor makes it mostly work when working with instances of Field but breaks for the type.

[michalpokusa]

I was able to pass the stubtests locally by typing e.g.

class IntegerField(Field):
    widget: _WidgetTypeOrInstance[NumberInput] | MediaDefiningClass

I changed _WidgetTypeOrInstance here to be a generic, but this works on non-generic version also.

Although the typing is not really correct in this case, because the widget is never a MediaDefiningClass instance, it still allows for better syntax highlighting than currently:

This is how the exact same code would look like with current typing:

[michalpokusa]

After a few hours of researching and testing multiple approaches, which mainly included trying to overcome problems with mypy, I finally came up with current version.

So, right now:

• the _WidgetTypeOrInstance is generic, and should type hint not only that field.widget is Widget, but also what type of widget subclass it is. I showed the screenshot in earlier comment Replace TypeAlias _ClassLevelWidgetT with descriptor _WidgetTypeOrInstance #2615 (comment)

• the generic descriptor accepts separate widget types for class and instance, why is that, because of subclasses of django.contrib.postgres.forms.ranges.BaseRangeField
  
  this is related to the Django implementation:

def __init__(self, **kwargs):
    if "widget" not in kwargs:
        kwargs["widget"] = RangeWidget(self.base_field.widget)

• I added all widgets that caused stubtest errors to allowlist.txt with annotation why they are there, and I added # type: ignore[assignment] in places where I override the typing for widget in field subclasses

Overall, all the tests pass, and I feel much more confident about this version rather than trying to make mypy happy, even when it is clearly wrong with its detection.

@sobolevn I would really appreciate your opinion on this, I believe it fits with the rest of the django-stubs codebase and that the use of allowlist.txt is justified in this case.