Repository: python-babel/babel
Pull request #947 (2023-01-18 11:16:49)

Keep @modifiers when parsing locales
Locale modifiers ("@variants") are described in the GNU gettext documentation like this:

> The ‘@variant’ can denote any kind of characteristics that is not
> already implied by the language ll and the country CC. […] It can also
> denote a dialect of the language, …

Wherein Babel previously would discard these, this patch stores the modifier information in the `Locale` objects, handling string representation accordingly.

Not implemented is the lookup of a meaningful description of modifiers, but instead — for now — an identity mapping is provided.

All tests pass. Furthermore, integration with `flask-babel` has been verified to work.

Resolves: #946
Signed-off-by: martin f. krafft <madduck@madduck.net>

madduck: @akx Thank you for your time. I addressed most of your comments, and left an open question in the above. Happy to proceed any way, just let me know what you prefer.
codecov[bot]: # [Codecov](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) Report
> Merging [#947](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) (b5d7749) into [master](https://codecov.io/gh/python-babel/babel/commit/2a4b78446f6553957d6181b9baad3ba2ecf4a2cc?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) (2a4b784) will **decrease** coverage by `0.64%`.
> The diff coverage is `100.00%`.

```diff
@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
- Coverage   90.48%   89.84%   -0.64%     
==========================================
  Files          24       24              
  Lines        4393     4403      +10     
==========================================
- Hits         3975     3956      -19     
- Misses        418      447      +29     
```


| [Impacted Files](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel) | Coverage Δ | |
|---|---|---|
| [babel/core.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvY29yZS5weQ==) | `96.87% <100.00%> (+0.09%)` | :arrow_up: |
| [babel/localtime/\_win32.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWx0aW1lL193aW4zMi5weQ==) | `25.92% <0.00%> (-37.04%)` | :arrow_down: |
| [babel/localtime/\_helpers.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWx0aW1lL19oZWxwZXJzLnB5) | `50.00% <0.00%> (-11.54%)` | :arrow_down: |
| [babel/localtime/\_\_init\_\_.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWx0aW1lL19faW5pdF9fLnB5) | `59.45% <0.00%> (-2.71%)` | :arrow_down: |
| [babel/localedata.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbG9jYWxlZGF0YS5weQ==) | `95.16% <0.00%> (-0.81%)` | :arrow_down: |
| [babel/messages/frontend.py](https://codecov.io/gh/python-babel/babel/pull/947?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel#diff-YmFiZWwvbWVzc2FnZXMvZnJvbnRlbmQucHk=) | `86.78% <0.00%> (-0.74%)` | :arrow_down: |

:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=python-babel)

madduck: Sorry for the failing tests, was due to the 4/5 tuple return depending on `modifier==None`. I am not actually very sure it's a good idea, but I see your point. Fixed now.

Patch:
diff --git a/babel/core.py b/babel/core.py
index 6b0c45d..bdd176a 100644
--- a/babel/core.py
+++ b/babel/core.py
@@ -168,6 +168,7 @@ class Locale:
         territory: str | None = None,
         script: str | None = None,
         variant: str | None = None,
+        modifier: str | None = None,
     ) -> None:
         """Initialize the locale object from the given identifier components.
 
@@ -181,6 +182,7 @@ class Locale:
         :param territory: the territory (country or region) code
         :param script: the script code
         :param variant: the variant code
+        :param modifier: a modifier (following the '@' symbol, sometimes called '@variant')
         :raise `UnknownLocaleError`: if no locale data is available for the
                                      requested locale
         """
@@ -192,10 +194,13 @@ class Locale:
         self.script = script
         #: the variant code
         self.variant = variant
+        #: the modifier
+        self.modifier = modifier
         self.__data = None
 
         identifier = str(self)
-        if not localedata.exists(identifier):
+        identifier_without_modifier = identifier.partition('@')[0]
+        if not localedata.exists(identifier_without_modifier):
             raise UnknownLocaleError(identifier)
 
     @classmethod
@@ -290,6 +295,11 @@ class Locale:
         >>> Locale.parse('und_AT')
         Locale('de', territory='AT')
 
+        Modifiers are optional, and always at the end, separated by "@":
+
+        >>> Locale.parse('de_AT@euro')
+        Locale('de', territory='AT', modifier='euro')
+
         :param identifier: the locale identifier string
         :param sep: optional component separator
         :param resolve_likely_subtags: if this is specified then a locale will
@@ -348,7 +358,11 @@ class Locale:
         # implement ICU like fuzzy locale objects and provide a way to
         # maximize and minimize locale tags.
 
-        language, territory, script, variant = parts
+        if len(parts) == 5:
+            language, territory, script, variant, modifier = parts
+        else:
+            language, territory, script, variant = parts
+            modifier = None
         language = get_global('language_aliases').get(language, language)
         territory = get_global('territory_aliases').get(territory, (territory,))[0]
         script = get_global('script_aliases').get(script, script)
@@ -359,7 +373,7 @@ class Locale:
         if script == 'Zzzz':
             script = None
 
-        parts = language, territory, script, variant
+        parts = language, territory, script, variant, modifier
 
         # First match: try the whole identifier
         new_id = get_locale_identifier(parts)
@@ -373,33 +387,40 @@ class Locale:
         # simplified identifier that is just the language
         likely_subtag = get_global('likely_subtags').get(language)
         if likely_subtag is not None:
-            language2, _, script2, variant2 = parse_locale(likely_subtag)
-            locale = _try_load_reducing((language2, territory, script2, variant2))
+            parts2 = parse_locale(likely_subtag)
+            if len(parts2) == 5:
+                language2, _, script2, variant2, modifier2 = parse_locale(likely_subtag)
+            else:
+                language2, _, script2, variant2 = parse_locale(likely_subtag)
+                modifier2 = None
+            locale = _try_load_reducing((language2, territory, script2, variant2, modifier2))
             if locale is not None:
                 return locale
 
         raise UnknownLocaleError(input_id)
 
     def __eq__(self, other: object) -> bool:
-        for key in ('language', 'territory', 'script', 'variant'):
+        for key in ('language', 'territory', 'script', 'variant', 'modifier'):
             if not hasattr(other, key):
                 return False
         return (
             self.language == getattr(other, 'language') and  # noqa: B009
             self.territory == getattr(other, 'territory') and  # noqa: B009
             self.script == getattr(other, 'script') and  # noqa: B009
-            self.variant == getattr(other, 'variant')  # noqa: B009
+            self.variant == getattr(other, 'variant') and  # noqa: B009
+            self.modifier == getattr(other, 'modifier')  # noqa: B009
         )
 
     def __ne__(self, other: object) -> bool:
         return not self.__eq__(other)
 
     def __hash__(self) -> int:
-        return hash((self.language, self.territory, self.script, self.variant))
+        return hash((self.language, self.territory, self.script,
+                     self.variant, self.modifier))
 
     def __repr__(self) -> str:
         parameters = ['']
-        for key in ('territory', 'script', 'variant'):
+        for key in ('territory', 'script', 'variant', 'modifier'):
             value = getattr(self, key)
             if value is not None:
                 parameters.append(f"{key}={value!r}")
@@ -407,7 +428,8 @@ class Locale:
 
     def __str__(self) -> str:
         return get_locale_identifier((self.language, self.territory,
-                                      self.script, self.variant))
+                                      self.script, self.variant,
+                                      self.modifier))
 
     @property
     def _data(self) -> localedata.LocaleDataDict:
@@ -424,6 +446,11 @@ class Locale:
         >>> Locale('zh', 'CN', script='Hans').get_display_name('en')
         u'Chinese (Simplified, China)'
 
+        Modifiers are currently passed through verbatim:
+
+        >>> Locale('it', 'IT', modifier='euro').get_display_name('en')
+        u'Italian (Italy, euro)'
+
         :param locale: the locale to use
         """
         if locale is None:
@@ -438,6 +465,8 @@ class Locale:
                 details.append(locale.territories.get(self.territory))
             if self.variant:
                 details.append(locale.variants.get(self.variant))
+            if self.modifier:
+                details.append(self.modifier)
             details = filter(None, details)
             if details:
                 retval += f" ({', '.join(details)})"
@@ -1115,9 +1144,12 @@ def negotiate_locale(preferred: Iterable[str], available: Iterable[str], sep: st
     return None
 
 
-def parse_locale(identifier: str, sep: str = '_') -> tuple[str, str | None, str | None, str | None]:
+def parse_locale(
+    identifier: str,
+    sep: str = '_'
+) -> tuple[str, str | None, str | None, str | None, str | None]:
     """Parse a locale identifier into a tuple of the form ``(language,
-    territory, script, variant)``.
+    territory, script, variant, modifier)``.
 
     >>> parse_locale('zh_CN')
     ('zh', 'CN', None, None)
@@ -1129,12 +1161,22 @@ def parse_locale(identifier: str, sep: str = '_') -> tuple[str, str | None, str
     ('en', '150', None, None)
     >>> parse_locale('en_us_posix')
     ('en', 'US', None, 'POSIX')
+    >>> parse_locale('it_IT@euro')
+    ('it', 'IT', None, None, 'euro')
+    >>> parse_locale('it_IT@custom')
+    ('it', 'IT', None, None, 'custom')
+    >>> parse_locale('it_IT@')
+    ('it', 'IT', None, None)
 
     The default component separator is "_", but a different separator can be
-    specified using the `sep` parameter:
+    specified using the `sep` parameter.
+
+    The optional modifier is always separated with "@" and at the end:
 
     >>> parse_locale('zh-CN', sep='-')
     ('zh', 'CN', None, None)
+    >>> parse_locale('zh-CN@custom', sep='-')
+    ('zh', 'CN', None, None, 'custom')
 
     If the identifier cannot be parsed into a locale, a `ValueError` exception
     is raised:
@@ -1144,14 +1186,13 @@ def parse_locale(identifier: str, sep: str = '_') -> tuple[str, str | None, str
       ...
     ValueError: 'not_a_LOCALE_String' is not a valid locale identifier
 
-    Encoding information and locale modifiers are removed from the identifier:
+    Encoding information is removed from the identifier, while modifiers are
+    kept:
 
-    >>> parse_locale('it_IT@euro')
-    ('it', 'IT', None, None)
     >>> parse_locale('en_US.UTF-8')
     ('en', 'US', None, None)
     >>> parse_locale('de_DE.iso885915@euro')
-    ('de', 'DE', None, None)
+    ('de', 'DE', None, None, 'euro')
 
     See :rfc:`4646` for more information.
 
@@ -1161,13 +1202,10 @@ def parse_locale(identifier: str, sep: str = '_') -> tuple[str, str | None, str
     :raise `ValueError`: if the string does not appear to be a valid locale
                          identifier
     """
+    identifier, _, modifier = identifier.partition('@')
     if '.' in identifier:
         # this is probably the charset/encoding, which we don't care about
         identifier = identifier.split('.', 1)[0]
-    if '@' in identifier:
-        # this is a locale modifier such as @euro, which we don't care about
-        # either
-        identifier = identifier.split('@', 1)[0]
 
     parts = identifier.split(sep)
     lang = parts.pop(0).lower()
@@ -1193,22 +1231,37 @@ def parse_locale(identifier: str, sep: str = '_') -> tuple[str, str | None, str
     if parts:
         raise ValueError(f"{identifier!r} is not a valid locale identifier")
 
-    return lang, territory, script, variant
-
-
-def get_locale_identifier(tup: tuple[str, str | None, str | None, str | None], sep: str = '_') -> str:
+    # TODO(3.0): always return a 5-tuple
+    if modifier:
+        return lang, territory, script, variant, modifier
+    else:
+        return lang, territory, script, variant
+
+
+def get_locale_identifier(
+    tup: tuple[str]
+    | tuple[str, str | None]
+    | tuple[str, str | None, str | None]
+    | tuple[str, str | None, str | None, str | None]
+    | tuple[str, str | None, str | None, str | None, str | None],
+    sep: str = "_",
+) -> str:
     """The reverse of :func:`parse_locale`.  It creates a locale identifier out
-    of a ``(language, territory, script, variant)`` tuple.  Items can be set to
+    of a ``(language, territory, script, variant, modifier)`` tuple.  Items can be set to
     ``None`` and trailing ``None``\\s can also be left out of the tuple.
 
-    >>> get_locale_identifier(('de', 'DE', None, '1999'))
-    'de_DE_1999'
+    >>> get_locale_identifier(('de', 'DE', None, '1999', 'custom'))
+    'de_DE_1999@custom'
+    >>> get_locale_identifier(('fi', None, None, None, 'custom'))
+    'fi@custom'
+
 
     .. versionadded:: 1.0
 
     :param tup: the tuple as returned by :func:`parse_locale`.
     :param sep: the separator for the identifier.
     """
-    tup = tuple(tup[:4])
-    lang, territory, script, variant = tup + (None,) * (4 - len(tup))
-    return sep.join(filter(None, (lang, script, territory, variant)))
+    tup = tuple(tup[:5])
+    lang, territory, script, variant, modifier = tup + (None,) * (5 - len(tup))
+    ret = sep.join(filter(None, (lang, script, territory, variant)))
+    return f'{ret}@{modifier}' if modifier else ret


Test patch:
diff --git a/tests/test_core.py b/tests/test_core.py
index 2a7e605..0023824 100644
--- a/tests/test_core.py
+++ b/tests/test_core.py
@@ -283,10 +283,12 @@ def test_parse_locale():
     assert (excinfo.value.args[0] ==
             "'not_a_LOCALE_String' is not a valid locale identifier")
 
-    assert core.parse_locale('it_IT@euro') == ('it', 'IT', None, None)
+    assert core.parse_locale('it_IT@euro') == ('it', 'IT', None, None, 'euro')
+    assert core.parse_locale('it_IT@something') == ('it', 'IT', None, None, 'something')
+
     assert core.parse_locale('en_US.UTF-8') == ('en', 'US', None, None)
     assert (core.parse_locale('de_DE.iso885915@euro') ==
-            ('de', 'DE', None, None))
+            ('de', 'DE', None, None, 'euro'))
 
 
 @pytest.mark.parametrize('filename', [