Babel 2.17 regression: TypeError on Windows for locale=''

[AA-Turner]

Reproducer:

import datetime

import babel
from babel.dates import format_date

date = datetime.date(2016, 2, 7)
try:
    print(format_date(date, locale=''))
except (ValueError, babel.UnknownLocaleError):
    print('Invalid Babel locale: ...')

Output:

PS> python bug.py
Traceback (most recent call last):
  File "...\bug.py", line 8, in <module>
    print(format_date(date, locale=''))
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\site-packages\babel\dates.py", line 705, in format_date
    locale = Locale.parse(locale or LC_TIME)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "...\Lib\site-packages\babel\core.py", line 350, in parse
    raise TypeError(msg)
TypeError: ("Empty locale identifier value: None\n\nIf you didn't explicitly pass an empty value to a Babel function, this could be caused by there being no suitable locale environment variables for the API you tried to use.",)

This is minimised from Sphinx's test suite, where we've newly been seeing failures only on Windows. After much attempted debugging, I noticed Babel had a recent release. With 2.16, the error message is printed, but in 2.17 a TypeError is raised.

I think this is a regression as locale='' is a valid type (str), but an invalid value. The new code uses locale or LC_TIME, which is None on Windows:

babel/babel/dates.py

Line 705 in b50a1d2

locale = Locale.parse(locale or LC_TIME)

A

[tomasr8]

Hi! This was changed in #1164. Locale.parse now explicitly rejects None with a TypeError though I agree that this should not happen when you explicitly pass a string to one of the format_ functions. What do you think @akx ?

[AA-Turner]

Right, the problem is that the change now uses the expression locale or LC_TIME, which short-circuits to LC_TIME, which is None on Windows (but presumably not None on Unix and macOS).

[tomasr8]

Indeed, this is still expected to fail, but it should IMO continue to fail with ValueError, not TypeError

[akx]

So the change was originally made (among other reasons) so passing an empty string at the top-most API level for a locale wouldn't give you a cryptic ValueError: expected only letters, got '' error:

$ env LC_TIME=vööh uvx --from=babel==2.16.0 python reproducer.py
[...]
ValueError: expected only letters, got ''

$ env LC_TIME=vööh uvx --from=babel==2.17.0 python reproducer.py
[...]
babel.core.UnknownLocaleError: unknown locale 'vööh'

Even more so, we now have a better error message (though now I noticed the error message is a 1-tuple of a string, because I'm a silly goose) when the value is not ambiently available either:

$ env LC_TIME= LANG= uvx --from=babel==2.16.0 python reproducer.py
[...]
ValueError: expected only letters, got ''

$ env LC_TIME= LANG= uvx --from=babel==2.17.0 python reproducer.py
TypeError: ("Empty locale identifier value: None...

I'm not 100% sure about making the latter exception a ValueError, because the value that gets eventually passed down to Locale.parse() is in this case indeed the wrong type – and we have a special case to keep raising a ValueError (for legacy compat, which obviously didn't exactly do the thing here...) if the value passed in to Locale.parse() is a string:

$ env LC_TIME= LANG= uvx --from=babel==2.17.0 python
>>> from babel import Locale
>>> Locale.parse("")
ValueError: ("Empty locale identifier value: ''...
>>> Locale.parse(None)
TypeError: ("Empty locale identifier value: None...

Trouble is of course we don't currently know whether our end-user is explicitly passing in a silly value, or if the silly value comes from the defaulting-from-environment behavior.

One option could be to make the higher-level functions do Locale.parse(locale or LC_TIME or _UNSET_LOCALE) (a sentinel value) (c.f. the current Locale.parse(locale or LC_TIME)) that we could explicitly detect and ValueError in Locale.parse.

🤔

Btw, @AA-Turner: does Sphinx's test suite explicitly/purposely unset locale-related environment variables, or should we also be wondering why no time locale gets detected on Windows?

[AA-Turner]

does Sphinx's test suite explicitly/purposely unset locale-related environment variables, or should we also be wondering why no time locale gets detected on Windows?

Not that I'm aware of. I ran the reproducer locally (Windows 11) and got identical output, so I imagine it is not specific to the test suite.

A

[AA-Turner]

Could you introduce a helper function?:

if locale:
    return locale
if LC_TIME:
    return LC_TIME
return '' if locale == '' else LC_TIME

[tomasr8]

One option could be to make the higher-level functions do Locale.parse(locale or LC_TIME or _UNSET_LOCALE)

But then if the passed locale happens to be None and LC_TIME is unset we'd raise a ValueError instead of TypeError, right? I don't think we want that.

Maybe explicitly checking for an empty string as Adam is suggesting would work?

[JoseG2003-g]

Hi, could me and my team tackle this issue ?

[tomasr8]

Sure, but first we need to figure out what the best course of action is here. @akx what do you think we should do?