translation.E005 / Should LANGUAGES_BIDI be a strict subset of LANGUAGES?

Visto 62 veces
Saltar al primer mensaje no leído

Matthias Kestenholz

no leída,
9 abr 2019, 6:22:399/4/19
a django-d...@googlegroups.com
Hello everyone,

The resolution of https://code.djangoproject.com/ticket/30241 in https://github.com/django/django/commit/4400d8296d268f5a8523cd02ddc33b12219b2535 introduced a new and in my opinion backwards incompatible requirement through the system checks framework.

Previously*, overriding LANGUAGES to restrict the list of available languages on a Django site worked. Now it's also necessary to override LANGUAGES_BIDI because there is a new system check (translation.E005) which verifies that there are no language codes in LANGUAGES_BIDI not in LANGUAGES as well.

It is my strong opinion that enforcing this does not provide any benefits and has the downside of forcing almost everyone who overrides LANGUAGES to now add a new setting to their project to override LANGUAGES_BIDI as well with unclear benefits.

Strictness is a good thing, but since LANGUAGES_BIDI is only ever used for membership checking (and never iterated over) I don't see the problem in keeping the default language codes from django/conf/global_settings.py in there even though some of those languages may not be available on a given installation anyway.

The report in https://code.djangoproject.com/ticket/30342 was closed as wontfix that's why I'm writing to the list. Also, since system checks generally aren't mentioned in the release notes users will not be informed ahead of time about this change when reading the release notes.

Anyone has any opinions to offer on this?

Thanks,
Matthias

* At least as long as I'm using Django, probably for 11 years or something.

Adam Johnson

no leída,
9 abr 2019, 7:28:119/4/19
a django-d...@googlegroups.com
Hi Matthias,

I can see why this is annoying. At least the resolution is easy by setting LANGUAGES_BIDI = []. I do agree that it should be in the release notes, although it's quite hard to miss a new system check's output when upgrading.

I see it was only at the last review of the PR ( https://github.com/django/django/pull/11060 ) that the check in question was upgraded from warning W005 to E005. I can see why a warning would make sense, but at the same time a warning is still a message to the user and resolved the same way, either by setting LANGUAGES_BIDI or adding it to SILENCED_SYSTEM_CHECKS.

I think I'm against removing the check, but pro anything that can make it easier for users such as yourself - perhaps something as simple as a suggestion in the error message to set LANGUAGES_BIDI = [] if you're not using RTL languages?

Thanks,

Adam

--
You received this message because you are subscribed to the Google Groups "Django developers (Contributions to Django itself)" group.
To unsubscribe from this group and stop receiving emails from it, send an email to django-develop...@googlegroups.com.
To post to this group, send email to django-d...@googlegroups.com.
Visit this group at https://groups.google.com/group/django-developers.
To view this discussion on the web visit https://groups.google.com/d/msgid/django-developers/CANvPqgAy_HytsvmnOpPtrCP_969QhoncJaPUe%3D6pN-VYWMVcjA%40mail.gmail.com.
For more options, visit https://groups.google.com/d/optout.


--
Adam

Matthias Kestenholz

no leída,
9 abr 2019, 8:01:449/4/19
a django-d...@googlegroups.com
Hi Adam

Thanks for your answer. I'll try to convince you (and others) that the error in question does not make sense, not even when downgraded to a warning.

There are system checks which provide great value such as django.contrib.admin checking whether templates, context processors etc. are configured correctly because it would be very confusing if the admin interface would not show messages after all.

There are other checks such as this one which do not protect the code from going wrong but that simply impose additional requirements to Django users.

I've reread the discussion in the pull request and haven't found a single reason why it is a good thing to start requiring that LANGUAGES_BIDI is specified explicitly. What is the rationale for requiring an explicit LANGUAGES_BIDI when all you want to do is restrict the overall list of languages for a single project (or the testsuite of many Django libraries for that matter)?

After all, the set of bidirectional languages does not change just because some of them aren't active inside a given project. And there is no ambiguity when inheriting LANGUAGES_BIDI from django.conf.global_settings and only overriding LANGUAGES.

Thanks,
Matthias


For more options, visit https://groups.google.com/d/optout.


--
www.feinheit.chbera...@feinheit.ch — +41 555 11 11 41

Tobias McNulty

no leída,
9 abr 2019, 8:04:029/4/19
a django-developers
I am curious, what is the reasoning behind disallowing LANGUAGES_BIDI from containing languages not in LANGUAGES in the first place?

If as Matthias says it is never iterated over, I'm unclear what problems this might cause, though I agree the fix is simple enough...

At least perhaps the last paragraph in the description for the LANGUAGES_BIDI setting (that is currently identical to a paragraph in the LANGUAGES description, "Generally, the default value should suffice. Only set this setting if you want to restrict language selection to a subset of the Django-provided languages.") could be modified to clarify this reasoning and its relationship to LANGUAGES? (What exactly this would say is not obvious to me at the moment...)

Tobias


Adam Johnson

no leída,
9 abr 2019, 8:06:519/4/19
a django-d...@googlegroups.com
Other ideas:
  • Bypass the check in the case that LANGAUGES_BIDI is the same as the one in global_settings - probably not the best idea, but would stop all projects having to set LANGUAGES_BIDI = [] to avoid the check
  • Change startproject so that LANGUAGES_BIDI is set up as the empty list - removes some "included batteries" but helps users make a more informed choice?
--
Adam

Tobias McNulty

no leída,
9 abr 2019, 8:19:269/4/19
a django-developers
Having thought about this a little more, I may have found in answer to my own question. :-)

The reasoning would seem to be that, by overriding LANGUAGES but not LANGUAGES_BIDI, you risk introducing an inconsistency between the language code *format* in these two settings (a la LANGUAGE_CODE and the original ticket that got this started). If that's correct, that seems fair to me (and worth keeping this check), along with an update to the docs to clarify this dependency and why it exists.

Tobias

Matthias Kestenholz

no leída,
9 abr 2019, 8:49:259/4/19
a django-d...@googlegroups.com
Checking that entries in LANGUAGES and LANGUAGES_BIDI are well formed certainly makes sense! I wouldn't want to back out this part of the change.

All settings come with a certain risk of inconsistencies or with unexpected behavior.

That being said, this particular change just seems so patronizing. As a team we have launched literally hundreds of projects based on Django (some of them using RTL languages) and released dozens of Django libraries and have never had a problem related to LANGUAGES_BIDI. And now, just because there's a possibility of an inconsistency I'll have to add the same line to each project when the whole thing can be avoided in the first place by keeping the meaning of LANGUAGES and LANGUAGES_BIDI orthogonal and just not introducing this dependency.

Matthias



For more options, visit https://groups.google.com/d/optout.


--

Mariusz Felisiak

no leída,
9 abr 2019, 9:24:599/4/19
a Django developers (Contributions to Django itself)
We can also remove LANGUAGES_BIDI from global_settings.py, check if LANGUAGES_BIDI is overridden in Settings.__init__() and if not then set it to a subset of RTL languages from LANGUAGES. This should solve all issues, IMO.

Mariusz Felisiak

no leída,
9 abr 2019, 9:25:249/4/19
a Django developers (Contributions to Django itself)
Sorry for technical description.

Mariusz Felisiak

no leída,
9 abr 2019, 10:21:149/4/19
a Django developers (Contributions to Django itself)

Mariusz Felisiak

no leída,
23 abr 2019, 6:07:4123/4/19
a Django developers (Contributions to Django itself)
I'm fine with removing this check and merge PR11244 prepared by Matthias. Any concerns?
Responder a todos
Responder al autor
Reenviar
0 mensajes nuevos