django-stubs
django-stubs copied to clipboard
django-stubs '1.10.0' introduced a regression when used with Django 3.2, that has persisted in all subsequent releases and currently still exists in 'master'
Bug report
What's wrong
It is indicated in the readme that all versions of django-stubs >= 1.9.0 support Django 3.2 (note: 1.12.0 is the latest django-stubs release at the time of writing). But, ever since #683 was merged and released as part of django-stubs 1.10.0, support for Django 3.2 breaks in at least one place. Specifically, I've noticed the stub for the .bulk_update() method on both the QuerySet and Manager no longer work for Django 3.2, because they were changed in 1.10.0 to return an int instead of None (as they did in version 1.9.0). However, this presents problems for Django 3.2 because, prior to Django 4.0, .bulk_update() did indeed return None. It wasn't until Django 4.0 that .bulk_update() started returning an int.
To quote from the Django 4.0 docs on .bulk_update():
This method efficiently updates the given fields on the provided model instances, generally with one query, and returns the number of objects updated: [emphasis mine]
>>> objs = [ ... Entry.objects.create(headline='Entry 1'), ... Entry.objects.create(headline='Entry 2'), ... ] >>> objs[0].headline = 'This is entry 1' >>> objs[1].headline = 'This is entry 2' >>> Entry.objects.bulk_update(objs, ['headline']) 2Changed in Django 4.0: The return value of the number of objects updated was added.
Now contrast to the Django 3.2 docs on .bulk_update():
This method efficiently updates the given fields on the provided model instances, generally with one query: [note the lack of any mention of any return value in the description, and the
int"2" is not being returned by the method, as contrasted to the Django 4.0 docs]>>> objs = [ ... Entry.objects.create(headline='Entry 1'), ... Entry.objects.create(headline='Entry 2'), ... ] >>> objs[0].headline = 'This is entry 1' >>> objs[1].headline = 'This is entry 2' >>> Entry.objects.bulk_update(objs, ['headline'])
Here is very simple code snippet that should demonstrate this behavior on Django 3.2 with django-stubs >= 1.10.0 installed:
from django.db import models
class MyQuerySet(models.QuerySet):
def bulk_update(
self,
objs: Iterable[models.Model],
fields: Iterable[str],
batch_size: Optional[int] = None,
) -> None:
super().bulk_update(objs, fields, batch_size)
This will raise the following error in mypy (again, using Django 3.2 and django-stubs >= 1.10.0):
error: Return type "None" of "bulk_update" incompatible with return type "int" in supertype "_QuerySet"
How should it be
mypy should not be raising an error here when a subclass of QuerySet/Manager is created that has an overriden .bulk_update() method annotated with None as the return type when Django 3.2 is being used, because the return type of .bulk_update() in Django 3.2 is indeed None and not an int.
I have a few ideas for ways this could be fixed, ordered by what I think the best option is (1) to the worst option (3):
-
Changing the return value of
.bulk_update()stub to beOptional[int], which should support both theDjango 3.2behavior (returnsNone) and theDjango >= 4.0behavior (returnsint). See below code snippet for a tiny proof of concept to show how usingOptional[int]as the return type in the base class allows subclasses to return eitherNoneorint, which results in nomypyerrors. This change can/should be released as part of version1.13.0(assuming that version is still intended to supportDjango 3.2, which I would hope so sinceDjango 3.2is currently the only supported LTS version ofDjangoand will be supported until at least 2024). I would also think this should include back-porting this fix to all versions ofdjango-stubs >= 1.10.0(ie. versions1.10.2(1.10.1already exists),1.11.1,1.12.1) so that they supportDjango 3.2as they purport to do in the readme.-
If
django-stubs 1.13.0is nearly ready to be released, and it is intended to still supportDjango 3.2, perhaps back-porting the change to previous releases could be considered non-essential. Ie. Just make the change and release it as part of1.13.0and encourage users to upgrade to that version if they run into this issue. However, we may want to back-port in any case to pin theDjangodependency insetup.pyfordjango-stubsversions>=1.10.0,<=1.12.0to be>=4.0, in which case, we might as well include the fix for this issue in the back-port. (This overlaps with option 2 below.)- Or, at the very least, update the readme to indicate that certain
Django 3.2annotations are broken fordjango-stubsversions1.10.x,1.11.0,1.12.0and hope that folks read it, but experience tells me that most folks won't. (This also overlaps with option 2 below.)
- Or, at the very least, update the readme to indicate that certain
-
from typing import Optional
class Foo:
def foo(self) -> Optional[int]:
return 123
class Bar(Foo):
def foo(self) -> None:
return None
class Baz(Foo):
def foo(self) -> int:
return 456
-
Updating the readme to indicate that
django-stubs >= 1.10.0only supportsDjango >= 4.0(which leaves the only version ofdjango-stubssupportingDjango 3.2to be1.9.0, which isn't ideal, since many bugs that were present in1.9.0have been fixed since then), or, at least, as mentioned above, add a disclaimer to the readme to indicate that some annotations forDjango 3.2do not work with these versions. ofdjango-stubs.- This could be improved somewhat by also back-porting new versions (ie. releasing versions
1.10.2(1.10.1already exists),1.11.1,1.12.1) to change the requirement boundaries set insetup.pyto indicate thatDjango >= 4.0is need for these versions. Without doing that, dependency management tools (pipfile,poetry, etc.) will still happily allow versions1.10.0,1.11.0, and1.12.0to be installed onDjango 3.2projects, causingmypyto fail when this issue is encountered.
- This could be improved somewhat by also back-porting new versions (ie. releasing versions
-
Do nothing and hope users of
Django 3.2figure out that they need to either downgrade todjango-stubs 1.9.0, or be forced to use# type: ignorewhenever they run into this issue using a newer version. In my experience, the more# type: ignores a repository has, the more likely it is to accidentally deploy broken code thatmypycould have caught. So, IMHO, I think this should be the last resort.
Since both options 1 and 2 are of the same, pretty low lift, I would recommend going with option 1 - though I am, of course, open to any argument for one of the other options - or any other options that I haven't thought of.
PR for merging option 1 into master is here: #1114
Also happy to back-port the changes to versions 1.10.1, 1.11.0, and 1.12.0 if this proposal is accepted.
System information
Re-created the issue on 2 systems, but it's pretty clear that this is just a direct incompatibility for the behavior of .bulk_update(), as Django 3.2 returns None, but the annotations found in django-stubs >= 1.10.0 indicate the method needs to return an int, which is only the behavior in Django >= 4.0.
System 1
- OS: MacOS
pythonversion:3.9.13djangoversion:3.2.15mypyversion:0.910(with1.9.0),0.942(with1.10.0and1.10.1), and0.950(with1.11.0and1.12.0)django-stubsversion:1.10.0,1.10.1,1.11.0, and1.12.0were all attempted (along with1.9.0, which works fine since this breaking change for users ofDjango 3.2wasn't introduced until1.10.0)django-stubs-extversion:0.5.0
System 2
- OS: Debian 10
pythonversion:3.9.5djangoversion:3.2.15mypyversion:0.910(with1.9.0) and0.931(with1.10.0,1.10.1,1.11.0, and1.12.0)django-stubsversion:1.10.0,1.10.1,1.11.0, and1.12.0were all attempted (as above,1.9.0was tested and works fine)django-stubs-extversion:0.3.0(with1.9.0and1.10.0) and0.4.0(with all versions>= 1.9.0, <= 1.12.0)