python-babel / flask-babel

i18n and l10n support for Flask based on Babel and pytz
https://python-babel.github.io/flask-babel/
Other
432 stars 159 forks source link

get_locale does not return a Locale object in some circumstances #213

Closed jdimmerman closed 1 year ago

jdimmerman commented 1 year ago

Per the documentation of get_locale, the return value should always be an instance of Locale or None. However, in cases where it returns the default locale, it is returning a string instead of an instance of Locale. In particular, I think that the each instance of locale = babel.default_locale should actually be locale = babel.instance.default_locale.

babel.instance.default_locale is defined by

    @property
    def default_locale(self) -> Locale:
        """The default locale from the configuration as an instance of a
        `babel.Locale` object.
        """
        return Locale.parse(get_babel().default_locale)

vs

babel.default_locale is a string because babel is an instance of BabelConfiguration.

This is new as of Babel 3.0.0. If helpful, I'm running on python 3.11.1.

SpacemanPaul commented 1 year ago

Confirming that I'm seeing this issue too.

Glandos commented 1 year ago

Note however that there is a discrepancy with the documentation and the code : https://github.com/python-babel/flask-babel/blob/master/flask_babel/__init__.py#L29

Is there a workaround for this, except monkeypatching?

TkTech commented 1 year ago

Note however that there is a discrepancy with the documentation and the code : https://github.com/python-babel/flask-babel/blob/master/flask_babel/__init__.py#L29

Is there a workaround for this, except monkeypatching?

The typing of what you linked to is correct. That is default_locale on the configuration object, which is a string.

jdimmerman commented 1 year ago

Agreed, none of the typing is wrong. Ideally get_locale() would have a return type annotation. The inferred return type is Locale | str | None rather than what I imagine it should be per the docs, Locale | None. I think that the fix is fairly straightforward (per my original issue notes) but I may be oversimplifying it

Glandos commented 1 year ago

Sorry for the noisy comment, and many thanks for the quick patch'n'release!