Avoid issues with LC_NUMERIC locale

[bouweandela]

🚀 Pull Request

Avoid a crash when importing the library as described in #435.

Closes #435.

[bouweandela]

The CI failures appear unrelated to this pull request.

[pp-mo]

@bjlittle can you spare this some attention? Looks simple enough to me, but I'm about to be unavailable for a couple of weeks..

[valeriupredoi]

hi folks, this would be nice if you guys had a mo to approve and get it in the new 3.11 🍺

[pp-mo]

I'm guessing the fails will be fixed by rebasing onto current main.
So I'll merge that in and we'll see how it goes...

[bouweandela]

All checks are green now 🚀

[pp-mo]

All checks are green now 🚀

Great work @bouweandela and sorry it has taken so long to respond !

One minor follow-on question : Is there any reasonable way of testing this to show it really works ?
( If not, I don't think we need to insist on it. )

[bouweandela]

Is there any reasonable way of testing this to show it really works ?

Testing that it really works would require installing a locale with the issue on the test system, e.g. nl_NL.utf8 and then running

export LC_NUMERIC='nl_NL.utf8'
python -c 'import locale; locale.setlocale(locale.LC_ALL, ""); import cf_units'

so that seems unpractical.

As an alternative, I tried adding a test in cf_units/tests/unit/test__udunits2.py that the locale is not set when reading the udunits database, but it's a bit tricky because the code is executed on import. When run separately this test seems to work, but then many other tests fail because I am unable to restore the original state after the test. Here is what I got so far:

def test_import(monkeypatch):
    """Test that locale is not set.

    The locale should not be set when creating a system object at module
    import time. See https://github.com/SciTools/cf-units/issues/435 for
    additional information.
    """

    def mock_read_xml(*args, **kwargs):
        assert locale.getlocale(locale.LC_NUMERIC) == (None, None)

    with monkeypatch.context():
        monkeypatch.setattr(_ud, "read_xml", mock_read_xml)
        locale.setlocale(locale.LC_ALL, "")
        importlib.reload(cf_units)

    importlib.reload(cf_units)

when running

pytest cf_units/tests/unit/test__udunits2.py::test_import cf_units/tests/unit/unit/test_Unit.py::Test_change_calendar::test_no_change

the second test fails, while separately they pass fine. Would you know how to restore the state after test_import?

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Please upload report for BASE (main@54b2819). Learn more about missing BASE report.
Report is 1 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #436   +/-   ##
=======================================
  Coverage        ?   90.50%           
=======================================
  Files           ?        6           
  Lines           ?      811           
  Branches        ?       93           
=======================================
  Hits            ?      734           
  Misses          ?       64           
  Partials        ?       13           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[pp-mo]

OK this sounds like too much trouble to test -- the relevant scope is not appropriate for a unit test anyway.
So, I have confirmed locally that this works + I'm just going to merge.