Add stubs for django-environ #14573
Add stubs for django-environ #14573
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
|
Thanks! I just want to note that the project doesn’t seem to be very actively maintained (the last commit was 7 months ago), so I wouldn’t expect type hints to be added to the library itself anytime soon. FTR: here’s a recent issue essentially complaining about the lack of type hints: joke2k/django-environ#567, and another one joke2k/django-environ#365 that might give some insight into the maintainer’s stance on adding them. |
This comment has been minimized.
This comment has been minimized.
|
According to mypy_primer, this change has no effect on the checked open source code. 🤖🎉 |
|
I noticed that you’ve added quite a few |
| @@ -0,0 +1,3 @@ | |||
| version = "0.12.*" | |||
| upstream_repository = "https://github.com/joke2k/django-environ" | |||
| requires = ["django-stubs"] | |||
There was a problem hiding this comment.
@donBarbos @brianschubert Any idea on how to fix the stub_uploader test which fails after adding the django-stubs requirement? If I am not mistaken it checks whether the original package requires django, which it doesn't (could be used without).
There was a problem hiding this comment.
I think we could pay attention to how this is done in other stubs for "django" packages, like django-filter METADATA.toml or django-import-export METADATA.toml
There was a problem hiding this comment.
Ah, yeah that could be a problem. I think you'll need to rollback the ImproperlyConfigured change and avoid any other django imports in the stubs. You can just add a note that ImproperlyConfigured re-exports the django exception when available
| def __call__( | ||
| self, var: _str, cast: type[_dict], default: _dict[_str, _str] | NoValue = ..., parse_default: _bool = False | ||
| self, var: _str, cast: type[_dict[Any, Any]], default: _dict[_str, _str] | NoValue = ..., parse_default: _bool = False | ||
| ) -> _dict[_str, _str]: ... |
There was a problem hiding this comment.
I noticed that you’ve added quite a few
Anytypes in generics. Just a gentle reminder that the use ofAnyis usually accompanied by a short comment explaining why it’s needed. If you’re not entirely sure about the type yet, you might consider usingIncompleteinstead. It could be a more suitable choice in such cases.
@donBarbos You are right. However, what I need is type[dict] of the builtin function. I think it's awkward, but pyright requires type arguments in that case. So I added the Any. Not sure if comments or replace with Incomplete makes more sense here.
There was a problem hiding this comment.
If you can't specify types just leave Incomplete, this is a good case.
Anyway in the future it could be improved (type Any is not very suitable as a placeholder)
Types for
django-environ.