-
-
Notifications
You must be signed in to change notification settings - Fork 122
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update django-stubs, mypy, pytest-mypy-plugins & fix tests #279
Update django-stubs, mypy, pytest-mypy-plugins & fix tests #279
Conversation
@@ -1,7 +1,7 @@ | |||
SECRET_KEY = "1" | |||
SITE_ID = 1 | |||
|
|||
INSTALLED_APPS = [ | |||
INSTALLED_APPS = ( | |||
"django.contrib.contenttypes", | |||
"django.contrib.sites", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
INSTALLED_APPS must be tuple for typecheck_tests, because DRF test suite does:
settings.INSTALLED_APPS += ('guardian',)
Better than ignoring IMO.
@@ -76,6 +76,7 @@ | |||
'Incompatible types in assignment (expression has type "AsView[GenericView]", variable has type "AsView[Callable[[HttpRequest], Any]]")', # noqa: E501 | |||
'Argument "patterns" to "SchemaGenerator" has incompatible type "List[object]"', | |||
'Argument 1 to "field_to_schema" has incompatible type "object"; expected "Field[Any, Any, Any, Any]"', | |||
'Argument "help_text" to "CharField" has incompatible type "_StrPromise"', |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I suppose we can make help_text: StrOrPromise
in a follow-up PR.
setup.py
Outdated
@@ -20,15 +20,15 @@ def find_stub_files(name): | |||
|
|||
dependencies = [ | |||
"mypy>=0.950", | |||
"django-stubs>=1.11.0", | |||
"django-stubs>=1.12.0", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm unsure how much I should bump these lower bound versions. Quite clearly these don't work in our CI, but in user projects they may work together?
If we want to be sure I suppose we should require mypy>=0.980 and django-stubs>=1.13.0.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we can do whatever is easier for us to maintain :)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah after some thought, I would bump both of these.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bumped these version requirements.
@sobolevn I'd like to get this merged, so djangorestframework-stubs is a step closer to being release-able again. (The other PR for mypy 0.990 can wait) |
Also by the way, I don't have privileges to merge or approve workflows in this repository yet. |
All these updates have to be done together, otherwise one dependency is incompatible with another in some way that fails in CI.
16b97a3
to
9e04621
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Great work, thank you!
All these updates have to be done together, otherwise one dependency is incompatible with another in some way that fails in CI.
Fixes #275, fixes #274