Add lots of missing argument & return type hints #1204
Conversation
intgr
force-pushed
the
add-lots-missing-func-hints
branch
from
7168897 to
cd3061b
Compare
|
I didn't fix all of the places -- there are tons more missing hints in |
There was a problem hiding this comment.
I love and hate PRs like this :)
I love them because you did a great work annotating so many files and functions!
I hate them because I am not able to review them properly :(
I will read thought it, but I won't check every param / annotation with the source code. Please, forgive that!
Discovered by setting mypy options disallow_untyped_defs, disallow_incomplete_defs.
intgr
force-pushed
the
add-lots-missing-func-hints
branch
from
cd3061b to
6dfbade
Compare
I agree, there's too much stuff all over the place. But the idea is that eventually we can enable |
| _M = TypeVar("_M", bound=Model) | ||
| _ModelFormT = TypeVar("_ModelFormT", bound=ModelForm) | ||
|
|
||
| class BaseGenericInlineFormSet(BaseModelFormSet[_M, _ModelFormT]): |
There was a problem hiding this comment.
Should we add it to our monkeypatching? Since it is generic now.
There was a problem hiding this comment.
BaseModelFormSet doesn't seem to be patched either.
|
CC @adamchainz I really need a second pair of eyes 🙏 |
There was a problem hiding this comment.
192 additions / 133 deletions is not too bad... had a quick look.
Is there any way to set the options in per-module settings in mypy.ini, for at least some modules?
| @@ -33,7 +34,7 @@ class ModelBackend(BaseBackend): | |||
| is_active: bool = ..., | |||
| include_superusers: bool = ..., | |||
| obj: Optional[Model] = ..., | |||
| ): ... | |||
| ) -> QuerySet[AbstractBaseUser]: ... | |||
There was a problem hiding this comment.
shouldn't this be with _AnyUser?
There was a problem hiding this comment.
Won't work
django-stubs/contrib/auth/backends.pyi:37: error: Type argument "Union[AbstractBaseUser, AnonymousUser]" of "_QuerySet" must be a subtype of "Model"
There was a problem hiding this comment.
This method does a DB query, so it only returns user models -- it can't return AnonymousUser.
Similar to how authenticate() returns AbstractBaseUser.
| @@ -10,4 +10,4 @@ from django.db.migrations.loader import MigrationLoader as MigrationLoader | |||
|
|
|||
| class Command(BaseCommand): | |||
| output_transaction: bool = ... | |||
| def execute(self, *args: Any, **options: Any): ... | |||
| def execute(self, *args: Any, **options: Any) -> Optional[str]: ... | |||
There was a problem hiding this comment.
did you check all management commands' execute / handle methods?
There was a problem hiding this comment.
Why "all management commands"? This is just one command: sqlmigrate
I used the same return as BaseCommand.
There was a problem hiding this comment.
I did check all functions that are missing return type annotations.
I didn't check all Command classes systematically in case they have differing return types, I think that's out of scope for this PR.
I tried, but couldn't get it to work. I suspect it might not work because the directory is called In any case I'll do a follow-up shortly that enables these checks globally. |
| @@ -45,7 +45,11 @@ class NestedObjects(Collector): | |||
| def nested(self, format_callback: Callable = ...) -> List[Any]: ... | |||
| def can_fast_delete(self, *args: Any, **kwargs: Any) -> bool: ... | |||
|
|
|||
| def model_format_dict(obj: Union[Model, Type[Model], QuerySet, Options[Model]]): ... | |||
| class ModelFormatDict(TypedDict): | |||
There was a problem hiding this comment.
I think that this does not exist in django, so it is _ModelFormatDict
There was a problem hiding this comment.
Ooops, sorry! I pushed this fix to the wrong branch. Will be fixed in #1206.
|
Thanks everyone! Major update! |
|
|
Discovered by setting mypy options
disallow_untyped_defs,disallow_incomplete_defs.