[Fix] Allow StrPromise to be used in exceptions #297
Conversation
XF-FW
force-pushed
the
fix/strpromise
branch
from
45ab664 to
a0323b9
Compare
|
Done. I'm getting a error however, which seems to be related to encode/django-rest-framework#8727. I'm unsure on how to proceed. |
rest_framework-stubs/exceptions.pyi
Outdated
| @@ -15,7 +16,7 @@ class ErrorDetail(str): | |||
| code: str | None | |||
| def __new__(cls, string: str, code: str | None = ...): ... | |||
|
|
|||
| _Detail: TypeAlias = str | list[Any] | dict[str, Any] | |||
| _Detail: TypeAlias = str | _StrPromise | list[Any] | dict[str, Any] | |||
There was a problem hiding this comment.
Don't import private types from django-stubs if it can be avoided. Use StrPromise from django_stubs_ext.
There was a problem hiding this comment.
Although I notice we currently don't have an explicit dependency on django-stubs-ext in setup.py. IMO that should that be added, thoughts @sobolevn?
There was a problem hiding this comment.
Also this could use the StrOrPromise type alias:
| _Detail: TypeAlias = str | _StrPromise | list[Any] | dict[str, Any] | |
| _Detail: TypeAlias = StrOrPromise | list[Any] | dict[str, Any] |
There was a problem hiding this comment.
django-stubs-ext is a dependency of django-stubs. I don't think we need to duplicate it 🤔
There was a problem hiding this comment.
Sorry to hijack this discussion but I can't find any specific documentation anywhere about this (I am digging through the issues to figure out new mypy errors after upgrading django-stubs). Just want to confirm that when I have code that uses gettext lazy strings I should use either StrPromise or StrOrPromise (depending on the specific case) from django-stubs-ext.
There was a problem hiding this comment.
Sorry for not getting back to you earlier.
StrOrPromise accepts both str and StrPromise.
StrPromise only allows StrPromse, but not regular str.
In nearly all cases StrOrPromise is the correct one.
|
The CI isssue will be hopefully fixed in #303 |
|
I rebased this and the CI now passes. |
|
I seem to have missed the notifications on this PR. I'll make the changes that @intgr has suggested, so please hold off merging for a bit. To clarify, it's replacing |
|
Yep 👍 |
rest_framework-stubs/exceptions.pyi
Outdated
| @@ -4,6 +4,7 @@ from typing import Any | |||
| from typing_extensions import TypeAlias | |||
|
|
|||
| from django.http import HttpRequest, JsonResponse | |||
| from django.utils.functional import _StrPromise | |||
There was a problem hiding this comment.
Oh my, looks like I have confused _StrPromise with _StrOrPormise as well 🤦
Naming!
|
Thanks! I ran into the same issue when doing the following: raise serializers.ValidationError(_('Foo'))This PR fixes this issue. Question regarding the use of |
I don't have strong feelings about that. In some other PR comments, sobolevn felt that it's not necessary. django-stubs aready depends on django-stubs-ext and as long as that remains, this will work. Let's leave it be. |
intgr
changed the title
I have made things!
Related issues
I didn't create an issue, but it's the mirror of typeddjango/django-stubs#1137, for this repo. There might be more places where the same should be applied, but this resolves my issue.