I18N locale per app resolving - documentation incorrect or bug?
Klaas van Schelven
According to
http://docs.djangoproject.com/en/1.2/howto/i18n/#using-translations-in-your-own-projects
and
http://docs.djangoproject.com/en/1.2/topics/i18n/deployment/#how-django-discovers-translations
At runtime, Django looks for translations by following this algorithm:
* First, it looks for a locale directory in the application directory
of the view that’s being called. If it finds a translation for the
selected language, the translation will be installed.
* Next, it looks for a locale directory in the project directory. If
it finds a translation, the translation will be installed.
* Finally, it checks the Django-provided base translation in django/
conf/locale.
However, this has not been my experience. I also do not see anything
in the django source code or tests that responds to the above.
In django/utils/translation/trans_real.py I see the following:
def translation(language):
[......]
for appname in settings.INSTALLED_APPS:
apppath = os.path.join(get_apppath(appname), 'locale')
if os.path.isdir(apppath):
res = _merge(apppath)
_merge (and deeper down "merge") is defined as a dictionary update in
the same file.
I see two problems here:
1] the documented behavior is not implemented
2] the implemented behavior has the opposite order-of-preference for
apps as the rest of Django. (Normally, the higher up in your
INSTALLED_APPS you put an app, the more likely it's templates, etc.
will be called. Here we're using a dict update, so the lowest app has
preference)
Am I missing something? I've done some grepping but couldn't find any
point where the "currently called view's app" is somehow related to
the used translation.
Should I file a bug? And if so: should we fix the documentation or the
code?
Klaas
Klaas van Schelven
I'm quite sure the docs are wrong by now.
Also: I think the documented behavior is not desirable. Changing
translations per-view makes for hard to debug once-in-a-while bugs.
This means the bug '2' is still a problem. I suggest either reversing
the loop
158 for appname in settings.INSTALLED_APPS:
or making the update of a 'merge' non-destructive.
Agreed? Should I move forward w/ tests, documentation update etc. in a
bug report?
On Dec 5, 11:21 am, Klaas van Schelven <klaasvanschel...@gmail.com>
wrote:
> Hi,
>
> According tohttp://docs.djangoproject.com/en/1.2/howto/i18n/#using-translations-i...
> andhttp://docs.djangoproject.com/en/1.2/topics/i18n/deployment/#how-djan...
Klaas van Schelven
http://code.djangoproject.com/ticket/14910
On Dec 5, 11:37 am, Klaas van Schelven <klaasvanschel...@gmail.com>
Klaas van Schelven
To be a bit more specific:
I've filed
http://code.djangoproject.com/ticket/14910
Added a test that proves the bug. Provided code that makes that test
green. Fixed the documentation. All including patches.
Could someone with sufficient authority move this forward? <include
trolling remark about the state of the community here to draw
attention to this ticket>
Much obliged,
Klaas
On Dec 16, 3:25 pm, Klaas van Schelven <klaasvanschel...@gmail.com>
> On Dec 5, 11:37 am,Klaasvan Schelven <klaasvanschel...@gmail.com>
>
>
>
>
>
>
>
> > Replying to myself here.
>
> > I'm quite sure the docs are wrong by now.
>
> > Also: I think the documented behavior is not desirable. Changing
> > translations per-view makes for hard to debug once-in-a-while bugs.
>
> > This means the bug '2' is still a problem. I suggest either reversing
> > the loop
>
> > 158 for appname in settings.INSTALLED_APPS:
>
> > or making the update of a 'merge' non-destructive.
>
> > Agreed? Should I move forward w/ tests, documentation update etc. in a
> > bug report?
>
Klaas van Schelven
Documentation incorrect: (incl. patch)
http://code.djangoproject.com/ticket/14910
Code incorrect: (patch w/ test)
http://code.djangoproject.com/ticket/14924
Please review,
Klaas
On Dec 19, 1:13 pm, Klaas van Schelven <klaasvanschel...@gmail.com>
wrote:
>
> To be a bit more specific:
>
Klaas van Schelven
Thanks for checking in 14910!
Could anyone take a serious look at
http://code.djangoproject.com/ticket/14924
It has a test that proves the problem and a relatively simple patch.
I'm doing a lot of "app overriding" and this bug is extremely annoying
in that case. It would be great if the fix could still make it to 1.3
thanks,
Klaas
On Dec 20, 2:22 pm, Klaas van Schelven <klaasvanschel...@gmail.com>
wrote:
>
Ramiro Morales
On Thu, Dec 23, 2010 at 7:43 AM, Klaas van Schelven
<klaasvan...@gmail.com> wrote:
> Hi all,
>
> Thanks for checking in 14910!
>
> Could anyone take a serious look at
> http://code.djangoproject.com/ticket/14924
>
> It has a test that proves the problem and a relatively simple patch.
>
> I'm doing a lot of "app overriding" and this bug is extremely annoying
> in that case. It would be great if the fix could still make it to 1.3
>
I'm pretty sure a design decision will be made about it before 1.3 gets
released. It is in the list of tickets I want to look at, and I know it is in
the radar of at least one core developer.
Just bear with us, we have a lot of tickets (some of them older
and/or uglier) to review and fix and one almost seven weeks to work on
bug killing before 1.3 goes gold.
Thanks for reporting, following and contributing to the solution of
this old issue, and for your patience.
--
Ramiro Morales
Klaas van Schelven
On Dec 23, 2:24 pm, Ramiro Morales <cra...@gmail.com> wrote:
> Klaas,
>
> On Thu, Dec 23, 2010 at 7:43 AM, Klaas van Schelven
>
Klaas van Schelven
I'll just keep patiently bumping my own bugreport, i.e.
http://code.djangoproject.com/ticket/14924
I also do have something new to add & request since last time on this
list. I'd like to have some discussion on my own comment below: I'd
like to have some input on the most desirable order of looking for
localization sources.
Having looked at the original tests the most desirable order appeared
to be: 1. Project path locale 2. Apps (in correct order) 3. Locale-
path 4. Django's own translations
I made a patch (submitted in the bugreport) which I have running in
some production code now, and I'm feeling this is not the most logical
order. (The order was originally established by looking at the
existing tests and code, and matching that order as closely as
possible while making sure the apps are evaluated in the correct, not
reverse, order).
However, in my mind, locale path should be the first to be evaluated.
The reason someone would put up a specific path for locales pretty
much has to be to override the existing behavior (which would be found
in project and apps). At least, that is how I use and want to use the
explicit locale path.
Input (discussion) on this is much appreciated. I'll gladly put in a
new patch once a decision is reached. If this is a separate issue, I'm
also fine on opening a separate ticket. (it could be argued that we
can move forward on the original bug report as a separate issue more
easily, since it's "obviously broken")
ciao,
Klaas
On Dec 23 2010, 5:00 pm, Klaas van Schelven
Ramiro Morales
<klaasvan...@gmail.com> wrote:
>
> Input (discussion) on this is much appreciated. I'll gladly put in a
> new patch once a decision is reached. If this is a separate issue, I'm
> also fine on opening a separate ticket. (it could be argued that we
> can move forward on the original bug report as a separate issue more
> easily, since it's "obviously broken")
>
Ok, had some time to look at this. These are the notes resulting from that:
a) The doc fix attached to #14910 to describe the current situation
and that got committed isn't correct. At no point of the catalog building
process a '``locale`` directory in the directory containing your settings
file.' is involved at all. And it doesn't mention the LOCALE_PATHS part.
The translation built is constructed by:
1. Adding the Django translations.
2. Updating it with the translations found in the paths listed in
settings.LOCALE_PATHS with the latter ones having greater
precedence.
3. Updating it with the translations found in the apps listed in
settings.INSTALLED_APPS with the latter ones having greater
precedence.
4. Updating it with the translation found in the locale/ dir under
the project dir. (this was before 3 before r12447)
this could be described in that way or by saying that the precedence
of the literals found while building the final unified translation is
(from higher to lesser):
* The translation found in the project locale/ dir
* the translations found in the apps listed in settings.INSTALLED_APPS
with the latter ones having higher precedence.
* The translations found in the paths listed in settings.LOCALE_PATHS
with the latter ones having higher precedence.
* The translations shipped with Django.
See also item d below.
b) I'm not sure I understand what converting the merge to a no
destructive update has to do with all this, I think the behavior of a
dictionary update() on the catalog being built is basic for the
translations overriding functionality.
c) Regarding reversing the priority of the translations in apps listed
in INSTALLED_APPS. Comparing it with the order followed when loading
templates isn't IMHO totally correct because template lookup has a
short circuit logic while translation loading has incremental
updating semantics.
I'm not saying I'm against the change (and against the change of the
precedence of the translation is LOCALE_PATHS) the but I think we
need to discuss further to decide if they really correct and if they
are worth the potential change in translated literals in existing
projects they might introduce.
d) I think the documentation text can be changed to make it clear that
no translation searching with short circuit is done but rather a
build of a unique translation by merging in/overriding the different
parts in the order documented. Also, the note about the
LocaleMiddleware can be dropped. Will take care of this soon.
Regards,
--
Ramiro Morales
Klaas van Schelven
>
> Ok, had some time to look at this.
>
> a) The doc fix attached to #14910 to describe the current situation
> and that got committed isn't correct. At no point of the catalog building
> process a '``locale`` directory in the directory containing your settings
> file.' is involved at all. And it doesn't mention the LOCALE_PATHS part.
>
What I meant with "a ``locale`` directory in the directory containing
your settings file." is "the project path". It is/was not clear to me
that Django thinks of the location of the file you specify when doing
--settings=some_settings as the "project path". This may lead to
unexpected behavior when you put your actual settings file in a
different location than the rest of your project files. Which led to
unexpected behavior for me.
Is this conflation of "the path to your specified settings file" with
"project path" the default Django behavior? I can imagine there's
other cases where a notion of a project path is used. If so: is this
documented somewhere?
Repeating "project path" at point 2 is basically me not paying
attention. Sorry about that.
> The translation built is constructed by:
>
> 1. Adding the Django translations.
> 2. Updating it with the translations found in the paths listed in
> settings.LOCALE_PATHS with the latter ones having greater
> precedence.
> 3. Updating it with the translations found in the apps listed in
> settings.INSTALLED_APPS with the latter ones having greater
> precedence.
> 4. Updating it with the translation found in the locale/ dir under
> the project dir. (this was before 3 before r12447)
>
> this could be described in that way or by saying that the precedence
> of the literals found while building the final unified translation is
> (from higher to lesser):
>
> * The translation found in the project locale/ dir
> * the translations found in the apps listed in settings.INSTALLED_APPS
> with the latter ones having higher precedence.
> * The translations found in the paths listed in settings.LOCALE_PATHS
> with the latter ones having higher precedence.
> * The translations shipped with Django.
>
> See also item d below.
>
> b) I'm not sure I understand what converting the merge to a no
> destructive update has to do with all this, I think the behavior of a
> dictionary update() on the catalog being built is basic for the
> translations overriding functionality.
(INSTALLED_APPS and LOCALE_PATHS) have higher precedence is non-
intuitive, we can implement a reversal of that order in one of two
obvious ways:
1] make the update non-destructive, and reverse the order you mention
above (1-4).
2] wrap both of the lists with reversed(), or something to that
effect.
I'm fine either way.
>
> c) Regarding reversing the priority of the translations in apps listed
> in INSTALLED_APPS. Comparing it with the order followed when loading
> templates isn't IMHO totally correct because template lookup has a
> short circuit logic while translation loading has incremental
> updating semantics.
a key part of your argument. Could you explain?
>
> I'm not saying I'm against the change (and against the change of the
> precedence of the translation is LOCALE_PATHS) the but I think we
> need to discuss further to decide if they really correct and if they
> are worth the potential change in translated literals in existing
> projects they might introduce.
guess and express the hope that others will chime in on the issue.
I have one clue that this will not lead to many backwards
compatibility issues. Which is: I haven't found anyone else
complaining or asking about the issue on Trac or elsewhere on the
internet.
Having said that, I think the proposed changes are "the right thing",
because:
1] on the matter of the reversed order within the lists
(INSTALLED_APPS and LOCALE_PATHS) this is the exact opposite of
Django's templating behavior, and for example the way staticfiles
works. So the behavior is unexpected, and will therefore lead to bugs.
Secondly, this means that if you have an application that depends on
the order of the apps for both templates and localizations, you're
hosed, because you can't put A before B and B before A at the same
time.
2] As to the higher preference for LOCALE_PATHS, I'll just repeat that
the reason someone would put up a specific path for locales pretty
in project and apps).
>
> no translation searching with short circuit is done but rather a
> build of a unique translation by merging in/overriding the different
> parts in the order documented. Also, the note about the
> LocaleMiddleware can be dropped. Will take care of this soon.
>
Klaas van Schelven
> Regards,
>
> --
> Ramiro Morales
Ramiro Morales
<klaasvan...@gmail.com> wrote:
>>
>> a) The doc fix attached to #14910 to describe the current situation
>> and that got committed isn't correct. At no point of the catalog building
>> process a '``locale`` directory in the directory containing your settings
>> file.' is involved at all. And it doesn't mention the LOCALE_PATHS part.
>>
>
>
> What I meant with "a ``locale`` directory in the directory containing
> your settings file." is "the project path". It is/was not clear to me
> that Django thinks of the location of the file you specify when doing
> --settings=some_settings as the "project path". This may lead to
> unexpected behavior when you put your actual settings file in a
> different location than the rest of your project files. Which led to
> unexpected behavior for me.
> Is this conflation of "the path to your specified settings file" with
> "project path" the default Django behavior? I can imagine there's
> other cases where a notion of a project path is used. If so: is this
> documented somewhere?
>
For me the project has always been the place where the settings file
is located. And yes, grep shows there are other places in Django that makes
this assumption but based in the structure created by startproject;
the project and application distinction and definitions are touched in part
one of the tutorial. I agree with that we can be a bit more specific here
and will refine that paragraph when updating the docs. Thanks for
the heads up.
>> b) I'm not sure I understand what converting the merge to a no
>> destructive update has to do with all this, I think the behavior of a
>> dictionary update() on the catalog being built is basic for the
>> translations overriding functionality.
>
> If we decide the he fact that the later latter elements in the lists
> (INSTALLED_APPS and LOCALE_PATHS) have higher precedence is non-
> intuitive, we can implement a reversal of that order in one of two
> obvious ways:
> 1] make the update non-destructive, and reverse the order you mention
> above (1-4).
> 2] wrap both of the lists with reversed(), or something to that
> effect.
Ok
>
> I'm not sure what you mean by "short circuit logic" and it seems to be
> a key part of your argument. Could you explain?
>
By short circuit logic I mean thay in the case of templates, one template is
looked up by traversing the paths in the documented order and when
one is found the lookup ends there.
>
> On the matter of backwards compatibility I can only make an educated
> guess and express the hope that others will chime in on the issue.
>
> I have one clue that this will not lead to many backwards
> compatibility issues. Which is: I haven't found anyone else
> complaining or asking about the issue on Trac or elsewhere on the
> internet.
>
> Having said that, I think the proposed changes are "the right thing",
> because:
>
> 1] on the matter of the reversed order within the lists
> (INSTALLED_APPS and LOCALE_PATHS) this is the exact opposite of
> Django's templating behavior, and for example the way staticfiles
> works. So the behavior is unexpected, and will therefore lead to bugs.
> Secondly, this means that if you have an application that depends on
> the order of the apps for both templates and localizations, you're
> hosed, because you can't put A before B and B before A at the same
> time.
>
> 2] As to the higher preference for LOCALE_PATHS, I'll just repeat that
> the reason someone would put up a specific path for locales pretty
> much has to be to override the existing behavior (which would be found
> in project and apps).
>
I'm leaning to agree in general with you regarding the directory precedence
order schema. But I'm still unsure we will be able to do this before 1.3. I will
attach a patch to the ticket and ask for opinions.
Thanks,
--
Ramiro Morales