Make ungettext_lazy really usable

[Claude Paroz]

In current Django code, ungettext_lazy is almost unusable, because you generally don't know the third argument (number) at lazy definition time. See #19158 and #19034 for use cases. It should be possible to pass the number argument at lazy resolution time.

Here's what we should be able to do:

class MyClass:
    # Note here the missing third argument
    string = ungettext_lazy("%(num)d string", "%(num)d strings")

    def resolve(self, number):
        # We hope here that the lazy 'magic' will be able to retrieve and pass number to ngettext
        return self.string % {'num': number}

[Claude Paroz]

Proof of concept

[Claude Paroz]

Different implementation (+docs and tests)

[Claude Paroz]

I've just uploaded a different implementation, a bit less hacky, including tests and documentation.

[Julien Phalip]

Looking at the patch, I think it makes sense. I'll spend some time to do a deeper review, but for now I just had one question: can we allow to have anonymous parameters? e.g.:

class MyClass:

    string = ungettext_lazy("%d string", "%d strings")

    def resolve(self, number):
        return self.string % number

[Claude Paroz]

Replying to julien:

Looking at the patch, I think it makes sense. I'll spend some time to do a deeper review, but for now I just had one question: can we allow to have anonymous parameters? e.g.:

No, this wouldn't work. I'm a bit reluctant to complicate this too much, I'd rather augment the documentation with limitations. But if you find some elegant way to achieve this, why not?

[Julien Phalip]

I've attached a patch with a different approach to allow it to work both with named and anonymous parameters. It doesn't modify functional.py but it does monkey-patches the proxy's __mod__ descriptor, which I concede is a bit of a code smell. Thoughts?

[Claude Paroz]

It's nice that it's working with anonymous parameter, but this will unfortunately not work properly when the string has more than one placeholder, like in "Hello %(name)s, you have %(num)d new messages". The .values()[0] part will arbitrarily pick one dictionary value, and in the case of unnamed parameters, you will pass the tuple to ungettext.

[Alexey Boriskin]

Another implementation

[Alexey Boriskin]

I've uploaded another implemtation, what do you think about it? There may be a confusing moment about when to call .resolve() and when not, and it also modifies the thing I probably should not be messed with - lazy()

[Claude Paroz]

There are interesting parts in your implementation. However, the fact that you have to call a method (resolve()) is creating a special case for translatable strings and defeats the ability to swap a ungettext_lazy object with any other string.

Basically, the use case of lazy gettext strings is to have a string defined at class/module level (defined at compilation time), and then use that string somewhere later at runtime. So it should be totally acceptable to have the string redefined somewhere in the code. Therefore, you don't know for sure at runtime if the string is an ungettext_lazy or any other string object. So having to call a special method on the string is a no-go for me.

[Alexey Boriskin]

Patch without resolve() method: ​https://gist.github.com/3945679

[Aymeric Augustin]

Reviewing the discussion and the patches above, I think Claude nailed the API in 19160-1.diff​ and Julien the implementation in 19160.smart-ngettext-lazy.diff​.

I suggest to adapt Julien's patch to Claude's API.

For anonymous parameters, I recommend to support only a single integer parameter. If no number argument is passed to ngettext_lazy, and later on the result is interpolated with a single integer value, that value should be used as the "count".

[Claude Paroz]

Aggregated patch

[Claude Paroz]

I mixed Julien's version with mine, mind to review?

[Alexey Boriskin]

Tiny fix of 19160-4.diff

[Alexey Boriskin]

I like the patch 19160-4.diff except the line key=number, which does nothing but renames global variable for use in the function which has local variable with the same name. Attached patch uses another name for the local variable and no rename is needed thereby

[Aymeric Augustin]

I reviewed the patch.

---

To sum up, the concept of *_lazy functions is to return a proxy object that stores the original function and its argument. When the proxy object is converted to a string, the function is evaluated. It can return different results if it takes into account a global (usually thread-local) variable — hire, the active language.

The complication here is that the number argument isn't known when the proxy object is built. We don't want to add an API to insert it (as acknowledged in comment 7). We want to extract it automatically when string interpolation is used on the proxy object (ie. the % operator, that invokes __mod__).

It would still be easy to inject the extra keyword argument, if keyword arguments weren't a "private" attribute of the proxy class (__proxy__.__kw)! "Private" attributes aren't actually private in Python; we can still set them with the _<classname>_<attrname> hack. Ugly — that's what we get for bending django.utils.functional.lazy.

(By the way, injecting a keyword argument is better than injecting a positional argument, because it works regardless of whether the previous arguments were positional or keyword arguments.)

The current implementation of lazy dates back to ​the merge of the i18n branch, more than 7 years ago. Maybe we could switch from double-underscore-prefixes to single-underscore-prefixes, alleviating the need for the _<classname>_<attrname> hack?

That won't change fundamentally the implementation — we're still hacking the proxy object's internals — but at least that'll avoid using a not-so-well-known Python hack.

---

There's a little bit of work on the patch, but it can be done by the committer:

• In the docs: .. versionadded:: 1.5 => .. versionchanged:: 1.6.
• The docs could use a little bit of copy-editing.
• In the tests: s = ungettext_lazy("%d good result", "%d good result") => the second arg should be a plural.

---

Last minute thought: if the ngettext function gets called with number=None — for instance because the lazy object is resolved without interpolation — is it easy to understand the traceback? We're explicitly introducing a default value of number=None here, possibly triggering complicated errors when the developer doesn't use one of the three supported syntaxes.

[anonymous]

I asked Jacob on IRC: he thinks it's possible to switch to single-underscore-prefixes in lazy.

[Simon Charette]

Do we need to test the single-underscore-prefixes lazy related switch before committing this?

[Alexey Boriskin]

Double underscores are there for a reason. If we'd switch to single underscore and use lazy() on function with the resultclass, which has clashing single underscore attribute, that attribute will be overwritten, which is no good.

[Aymeric Augustin]

We talked a lot about this ticket on IRC. The latest discussion concluded with:

2013-01-03 23:41:27 <void> mYk: I wonder if there is a way to rewrite lazy() with __get__/__set__ descriptors instead of all that copying
2013-01-03 23:43:10 <mYk> it would probably be cleaner and faster...
2013-01-03 23:45:24 <void> mYk: yes. but I am not sure if there are some hidden pitfalls that way

This could help avoid the _<classname>_<attrname> hack.

[Alexey Boriskin]

Another approach to solve this: ​https://gist.github.com/4605085 (To use this, we need part of #19642 about __mod__ solved, patch is at ​https://gist.github.com/4605065)

[Alexey Boriskin]

I've sent a pull request with code cleaned up a bit: ​https://github.com/django/django/pull/684

[Aymeric Augustin]

This looks way better than the previous attempts. It's probably RFC — it just needs running the tests on Python 2 and 3 and proof-reading carefully.

[Claude Paroz]

Thanks Alexey! Tests have been successfully run on Python 2.6/SQLite and Python 3.2/SQLite.

To prevent deprecation warnings, self.assertRaisesRegexp should be changed to six.assertRaisesRegex. Apart from that, it looks very nice.

[Aymeric Augustin <aymeric.augustin@…>]

In 3f1a0c0040b9a950584f4b309a1b670b0e709de5:

Fixed #19160 -- Made lazy plural translations usable.

Many thanks to Alexey Boriskin, Claude Paroz and Julien Phalip.

[Alexey Boriskin]

Our new ungettext_lazy function lacks support for pretty trivial, but widespread usecase, when we don't have number in a string we're translating.
For example, I tried to replace lambda-hack in django.utils.timesince (#19704), and it was rather confusing to use ungettext_lazy('year', 'years', 'n') % {'n': count} instead of just ungettext_lazy('year', 'years') % count

[Aymeric Augustin]

I suggest to catch TypeError, and if it's raised, retry without interpolation.

[Alexey Boriskin]

​https://github.com/django/django/pull/692 - pull request implementing that try/catch for TypeError

[Aymeric Augustin <aymeric.augustin@…>]

In ace9d4efc3e6e0b07fd94b039d62e0d32c81ed3f:

Made ungettext_lazy usable for messages that do not contain the count.

Fixed #19160 (again). Thanks Alexey Boriskin.