Fix regression in '.bulk_update()' annotations for Django 3.2

[javulticat]

Related issues

Refs #1113

Description

This PR solves a regression for users of Django 3.2 introduced in django-stubs 1.10.0 (via #683) that changes the return type of .bulk_update() to int, which is only returned by Django 4.0 and later. In Django 3.2, the .bulk_update() method returns None, so mypy will throw an error if a Django 3.2 user attempts to override the .bulk_update() method in a subclass and annotate its return type as None, even though this is the correct return type for this method in Django 3.2.

This PR changes the return type to be Optional[int] (which, under the hood, is Union[None, int]), so users of both Django 3.2 and Django 4.0 should be able to override .bulk_update() and annotate its proper return type for their Django version (None for 3.2 and int for 4.0+) without having mypy throw an error. Without creating separate releases for Django 3.2 and Django 4.0 (which I imagine would be a significant undertaking), this seems to be the best solution available.

Also, while I was editing the type annotations for .bulk_update() for this fix, I noticed a divergence in the stubs for QuerySet's .bulk_update() method and Manager's .bulk_update() method. It seems that #868 changed the type annotation of .bulk_update()'s fields argument from Sequence[str] to Iterable[str]. However, this was only done on the QuerySet .bulk_update() method, but not the Manager's. So, I also changed the fields argument for the Manager's .bulk_update() method from Sequence[str] to Iterable[str] to make it match the QuerySet's method, since the type signatures of the two methods should be identical.

Details

See the issue I opened (#1113) for a much more detailed breakdown of this issue. But at a high-level, just looking at the Django docs for 3.2 and 4.0, respectively, should illuminate the problem. For easy reference, here are relevant screenshots from the different versions of the docs:

Screenshots

Django 3.2

Django 4.0

Open questions

As I recommended in #1113, not only do I think this should be merged and released with 1.13.0, it may also be worth considering back-porting this change to versions 1.10.1, 1.11.0, and 1.12.0. Because, without doing that, they don't fully support Django 3.2, despite the readme saying otherwise. If there is agreement on this, I am happy to also open PRs to back-port this to those 3 versions. Then they can presumably be released as 1.10.2 (there is already a 1.10.1), 1.11.1, and 1.12.1, respectively.

[flaeppe]

By changing to Optional[int] we've gone from having a correct return type for more recent versions to have an incorrect return type for all versions (don't think bulk_update has ever returned Optional[int]).

Could it work to wrap the bulk_update definition in an if-statement that branches on the django version instead?

[javulticat]

That's a great idea, and actually what I attempted first, but I could not get it to work, and I'm having a hard time finding any examples on the internet that show that conditional type hinting logic based on the installed Django version is indeed possible. For example, I don't see any other example of that kind of logic occurring in this codebase, and, from what I understand, that has historically been the reason why each version of django-stubs has only ever claimed/attempted to support a single version of Django - because, AFAIK, mypy is specifically only able to handle conditional logic in a few limited cases:

• sys.version_info
• sys.platform
• typing.TYPE_CHECKING
• an in-scope variable named MYPY

(Source: https://mypy.readthedocs.io/en/stable/common_issues.html#python-version-and-system-platform-checks)

But if you want it to handle conditional logic based on anything else, such as the installed Django version, you're out of luck (from what I understand).

For example, mypy is fine with this (which isn't what we want, but just showing an example of how it can handle conditional logic based on one of the above things mypy explicitly says it can handle):

import sys

if sys.version_info < (4,):
    def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> None: ...
else:
    def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> int: ...

Results in no mypy error.

But, if instead we do what we actually want to do, which is have a conditional on the Django version, not the Python version, mypy won't accept it:

import django

if django.VERSION < (4,):
    def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> None: ...
else:
    def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> int: ...

We get this mypy error: error: All conditional function variants must have identical signatures

I've tried all sorts of combinations with the things mypy allows conditional logic on with no luck:

• if sys.version_info and django.VERSION < (4,):
• if sys.platform and django.VERSION < (4,):
• if typing.TYPE_CHECKING and django.VERSION < (4,):

• MYPY = True
  if MYPY and django.VERSION < (4,):

All result in the same mypy error: error: All conditional function variants must have identical signatures

I've also tried nesting the conditionals for each of the above cases too, in the hopes that it would somehow allow mypy to process everything under the conditional - for example:

if typing.TYPE_CHECKING:
    if django.VERSION < (4,):
        def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> None: ...
    else:
        def bulk_update(self, objs: Iterable[_T], fields: Iterable[str], batch_size: Optional[int] = ...) -> int: ...

But, again, same error each time: error: All conditional function variants must have identical signatures

I'm not having much luck in finding any information about this elsewhere (StackOverflow, Google, etc.) where someone has used mypy successfully in handling conditional logic outside of the 4 special cases it specifically supports, unfortunately. Hopefully someone more knowledgeable than me can come along and show me what I'm missing on how this can be possible 🙃

[flaeppe]

I see. This feels somewhat of a limitation that could be resolved through mypy. But I'm not at all sure.

What I'm thinking is that if mypy exposes some sort of hook for plugins to declare these sort of constants by themselves.

Until we can find support for declaring 2 versions of the bulk_update signature I'd lean towards having django-stubs exporting types for newer versions of Django. But perhaps that's just me.