Return Promise for lazy functions.

[PIG208]

Django uses a proxy method for lazy translation functions that actually returns instances of Promise instead of str. To be accurate, we should change the signatures for these functions.

Related issues

• Closes #688
• Refs #688

[PIG208]

Hi, I have updated the PR. I think it is probably not necessary to make Promise generic at this point, having these methods should be sufficient.

[PIG208]

There is still a problem with this. Promise proxied other methods of resultclasses, so we probably want those be available on the Promise object as well. Otherwise, these methods won't be usable. I will try to update the plugin for this.

[PIG208]

This is my first time working on a plugin. I hope this implementation does not go horribly wrong. Another feature that I'm planning to add is to being able to determine what attributes are available on Promise[_ResultClassT] by calculating the mro of the proxied object.

Which hook should I use, get_customize_class_mro_hook, get_attribute_hook (combined with adding __getattr__ to Promise) or get_method_hook? Any help will be appreciated!

[PIG208]

I believe that this implementation should be good enough for another review. @sobolevn could you take a look at this?

I ditched the generic approach and reimplement this with a simpler plugin handling the use case for lazystr (and *_lazy as well) suggested by @andersk . This is tested with some additional test cases and the entire codebase of Zulip (edit, there still seems to be some issues to be resolved. Will update soon ). The earlier commits are left unchanged.

Looks like the CI is still failing for another unrelated issue. Will look into this in a separate PR probably. Otherwise, I think we are good to go.

[PIG208]

@sobolevn Hi! The PR has been rebased and the CI is passing. I think it is ready for review.

[hannseman]

This PR causes errors when passing gettext_lazy strings to parameters expecting strings like verbose_name and help_text on model fields:

error: Argument "verbose_name" to "PositiveBigIntegerField" has incompatible type
"_StrPromise"; expected "Optional[str]"  [arg-type]
            null=True, blank=True, verbose_name=_("number")

(where _ is defined as from django.utils.translation import gettext_lazy as _)

[sobolevn]

Please, open a new issue. I am not releasing new version yet, we have time to fix it.

[terencehonles]

@PIG208 was there a reason _StrPromise

https://github.com/typeddjango/django-stubs/blob/fa972f1632fb69861c913f2bf359fcf4d2284d7a/django-stubs/utils/functional.pyi#L31-L46

Is a subclass of Sequence[str] and not str itself? A string is slice-able so I understand the thought process at a high level, but lazystr expects/returns a string not a sequence of strings and I'm not sure why this was made this generic / explicit. This has caused a lot of downstream code (even in this codebase) to need to declare everything as StrOrPromise. While this doesn't seem like that big of a lift, I'm not sure it should be needed at all.

I'm raising this question here instead of creating a separate issue since I figured I'd ask about the intent before completely suggesting this should change (maybe there was a reason I am not aware of).

It seems like Promise should have been made generic and returns and possibly subclasses the type it is so that callers can treat a promise and the object itself interchangeably.

@sobolevn not sure if you have any thoughts or would prefer this to be a separate issue.

[PIG208]

@terencehonles Thanks for bringing this up!

_StrPromise is a subclass of Sequence[str] because this is how builtins.str is typed in typeshed. _StrPromise is defined only because operators don't work well with mypy plugins, while we want methods on a regular str to be all available. These are just the implementation details.

The point of having _StrPromise is to avoid using them with regular str interchangeably. This can cause real bugs when dealing with third-party libraries, because Promise is not str.

We did not make Promise generic because that would require more work to implement a full-blown mypy plugin to support arbitrary type arguments, while the most prevalent use case of Promise is with lazily evaluated strings.

[andersk]

See also https://github.com/typeddjango/django-stubs/pull/1139#issuecomment-1232167698 for concrete examples of what would go wrong if we incorrectly told mypy that _StrPromise is a subclass of str.

[terencehonles]

ok, thanks for the explanation.

The point of having _StrPromise is to avoid using them with regular str interchangeably. This can cause real bugs when dealing with third-party libraries, because Promise is not str.

A Promise[str] seems like it should be mostly considered a string and I'm not sure I agree or have run into an issue that this comment refers to, but I understand it would fail an isinstance(...) check and for mypy and potential future compilation would be concerned that would be an issue.

I had believed that Django's Promise implementation should be a more or less drop in replacement for the object it's promising since I thought it's blockingly coerced into the object when it's used, but it sounds like there's been some discussion of this and I will check out the comment @andersk pointed to :+1: