Add explicit overload to Field subclasses

[Viicos]

In #1264 (comment), and to try working on bringing django-stubs and django-types together, I suggested a workaround that would allow type checkers to natively infer _ST and _GT without the need for a plugin.

Unfortunately, for it to work correctly it would most probably require higher kinded type vars (my current example doesn't play well with subclasses, and I can't use Self as it can't be parametrized).

This is an experimental test, and as you can see it adds a lot of verbosity. But this removes the need to a plugin to parametrize the type variables, and it support nullable fields.

Would you accept such a change? I'm ok if this isn't accepted, considering how much it adds

[sobolevn]

I like the idea. Several comments:

1. Please, delete the plugin code, so we can be sure that your changes are enough to pass our CI
2. Looks like stubstest is not happy with some of the changes. Please, fix or ignore them :)

[Viicos]

Great to hear! I'm also experimenting on another solution, but facing some issues (see microsoft/pyright#6914).

If this end up working, I think it might be preferable as it would avoid redefining all overloads, + people could easily subclass field classes.

Edit:

This alternative solution will require some fixes on both pyright and mypy, but it looks promising. On the pyright side, it is currently being investigated implemented. On the mypy side, it would require python/mypy#3737, and probably python/mypy#11366 (a fix that you created 😄). So I think it's best to wait a bit for this one.

[Viicos]

After looking at the related plugin code, I don't know if removing _pyi_private_set/get_type support is the best way. The thing is the following function:

django-stubs/mypy_django_plugin/transformers/fields.py

Lines 133 to 153 in 1735d9e

	def set_descriptor_types_for_field(
	ctx: FunctionContext, *, is_set_nullable: bool = False, is_get_nullable: bool = False
	) -> Instance:
	default_return_type = cast(Instance, ctx.default_return_type)
	is_nullable = False
	null_expr = helpers.get_call_argument_by_name(ctx, "null")
	if null_expr is not None:
	is_nullable = parse_bool(null_expr) or False
	# Allow setting field value to `None` when a field is primary key and has a default that can produce a value
	default_expr = helpers.get_call_argument_by_name(ctx, "default")
	primary_key_expr = helpers.get_call_argument_by_name(ctx, "primary_key")
	if default_expr is not None and primary_key_expr is not None:
	is_set_nullable = parse_bool(primary_key_expr) or False
	set_type, get_type = get_field_descriptor_types(
	default_return_type.type,
	is_set_nullable=is_set_nullable or is_nullable,
	is_get_nullable=is_get_nullable or is_nullable,
	)
	return helpers.reparametrize_instance(default_return_type, [set_type, get_type])

Is being called before mypy actually infers the return type of a call to FooField, with the overloads provided in this PR. So I can't really take the "native" mypy solving logic into account. Besides, we can see primary key fields are special cased, so removing the plugin related logic here isn't trivial.

I also saw a couple hits of public code making use of these attributes on field subclasses.

The idea here would be to add tests without the plugin added in the mypy config, and assert that they work. I've tried making use of the parametrized feature of the pytest plugin, but looking at the CI it doesn't seem to play well.

I might take a second look and see if it's in fact possible to remove the plugin code

[Viicos]

Having explicit assignments feels a bit weird in a stub file and was causing issues when testing without the plugin (had error code var-annotated)

[Viicos]

... however I can't explicitly annotate permissions: models.ManyToManyField[..., ...] as the second type var is solved by the plugin to a class that doesn't exist in stubs nor at runtime

[flaeppe]

You can have a look here how it can be done: https://github.com/typeddjango/django-stubs/pull/2214/files#diff-0236ca9e909bc4743fa03e6653fc33adbaaef02a34d8698501a63a59ad624f1fR42-R51

[Viicos]

...So I had to disable the error code anyway when testing without the plugin

[Viicos]

I rebased everything to have commits easier to review. CI is now passsing, except stubtests, as I had to make arguments following null as kw-only. This could be added to the allowlist but doesn't seem like a great solution. Any ideas how this could be handled?

I created a pull request @ mypy to add a --ignore-positional-only argument, might be more robust

[intgr]

What's the status here, is it waiting for review?

@sobolevn since you commented, do you want to review this?

[Viicos]

What's the status here, is it waiting for review?

I think #1900 (comment) needs to be resolved in some way. Even if not really ideal, the linked mypy open PR seems to be the best option.

A slightly more elegant solution to this PR is described here, but unsupported in mypy currently.

This PR is also imo not a blocker for the next release

[Viicos]

I think the additions from #2048 is what's breaking the when_default_for_primary_key_is_specified_allow_none_to_be_set test case.

[flaeppe]

I was playing around with getting rid of _ST and _GT in fields from related.pyi (ForeignKey and OneToOneField) and got stuck on python/mypy#14764 in regards to overloading on the null argument.

Now I'm thinking that python/mypy#14764 must be a problem for these changes too?