Fix leap second update when using a non english locale #11062

saimn · 2020-11-19T22:39:23Z

I'm working on a documentation, and for every build I see the leap second update with an expiration warning:

Downloading https://hpiers.obspm.fr/iers/bul/bulc/Leap_Second.dat
|==========================================================| 1.3k/1.3k (100.00%)         0s
Downloading https://www.ietf.org/timezones/data/leap-seconds.list [Done]
WARNING: IERSStaleWarning: leap-second file is expired. [astropy.utils.iers.iers]

I already saw this in the past, so this is not new, but I never took the time to investigate... I can reproduce with various documentations (1, 2, 3), but the weird thing is that there is no warning when importing directly astropy.

Putting some prints in LeapSeconds.auto_open gives this for a normal import:

❯ python -c "import astropy.coordinates.earth_orientation"
open erfa
expires: 2017-06-30 00:00:00.000
open /home/simon/dev/astropy/astropy/utils/iers/data/Leap_Second.dat
expires: 2021-06-28

which is fine. And this for a Sphinx build:

open erfa
expires: 2017-06-30 00:00:00.000
open /home/simon/dev/astropy/astropy/utils/iers/data/Leap_Second.dat
ERROR: time data '28 June 2021' does not match format '%d %B %Y'
open https://hpiers.obspm.fr/iers/bul/bulc/Leap_Second.dat
ERROR: time data '28 June 2021' does not match format '%d %B %Y'
open https://hpiers.obspm.fr/iers/bul/bulc/Leap_Second.dat
Downloading https://hpiers.obspm.fr/iers/bul/bulc/Leap_Second.dat
|==========================================================| 1.3k/1.3k (100.00%)         0s
ERROR: time data '28 June 2021' does not match format '%d %B %Y'
open https://www.ietf.org/timezones/data/leap-seconds.list
Downloading https://www.ietf.org/timezones/data/leap-seconds.list [Done]
ERROR: time data '28 June 2021' does not match format '%d %B %Y'
WARNING: IERSStaleWarning: leap-second file is expired. [astropy.utils.iers.iers]

So when running with Sphinx, there is a parsing error with the Leap_Second.dat file, which lead to this:

ipdb> Time.strptime('28 June 2021', '%d %B %Y', scale='tai', format='iso', out_subfmt='date')
*** ValueError: time data '28 June 2021' does not match format '%d %B %Y'

%B is the Locale's full month name, but in both case my locale if fr_FR.
Diving deeper leads to _strptime.py where _getlang returns None in one case and fr_FR in the other, the latter resulting in a regex with French month names 🎉
And I guess Sphinx is setting LC_TIME to the default locale, which triggers the error in _strptime.py.

pllim · 2020-11-19T22:51:40Z

How interesting! There should be a change log. Not sure about a test, I'll let @mhvk decide.

saimn · 2020-11-19T23:14:43Z

Hmm, this is not yet done... Putting _set_locale in _read_leap_seconds as I did initially causes a race condition somewhere 😱 (the leap second tests get stuck). And moving it to Time.strptime works, but I don't know why and don't think we want this here, though having an option to force the locale in Time.strptime could be an option ?

mhvk

Wow, sorry to have caused what must have been a hard bug to track down! I wish one could just tell strptime to use a given locale, but for some reason that is not possible.

I think we need to be a bit careful about the solution (though perhaps the worry about threading problems is overblown in this case...)

mhvk · 2020-11-20T18:53:30Z

.github/workflows/ci_workflows.yml

@@ -70,7 +70,7 @@ jobs:
        python-version: ${{ matrix.python }}
    - name: Install language-pack-de and tzdata
      if: startsWith(matrix.os, 'ubuntu')
-      run: sudo apt-get install language-pack-de tzdata
+      run: sudo apt-get install language-pack-de language-pack-fr tzdata


Might it make sense to use German for the tests so that we don't have to install another language pack (they're fairly large so this will slow down things).

There is another test using it (

astropy/astropy/wcs/tests/test_wcsprm.py

Lines 865 to 876 in a68f456

def test_locale():

try:

with _set_locale('fr_FR'):

header = get_pkg_data_contents('data/locale.hdr',

encoding='binary')

with pytest.warns(FITSFixedWarning):

w = _wcs.Wcsprm(header)

assert re.search("[0-9]+,[0-9]*", w.to_header()) is None

except locale.Error:

pytest.xfail(

"Can't set to 'fr_FR' locale, perhaps because it is not installed "

"on this system")

), an old wcs bug, and it's easier for me to test locally that it works as expected ;)

Maybe we can get rid of german instead!

mhvk · 2020-11-20T19:04:21Z

astropy/utils/iers/iers.py

-                                            scale='tai', format='iso',
-                                            out_subfmt='date')
+                    try:
+                        locale.setlocale(locale.LC_TIME, 'C')


Hmm, locale is quite explicit about not being thread safe. There is a suggested solution at https://stackoverflow.com/questions/18593661/how-do-i-strftime-a-date-object-in-a-different-locale, though I wonder if we are not better off just parsing it ourselves (as also suggested in various places). I think with a bit smarter regex this may just be possible........

Yes indeed, the thing is that when using _set_locale here the tests are stuck in a deadlock...
Maybe another soution to parse the date would be better:

If, when coding a module for general use, you need a locale independent version of an operation that is affected by the locale (such as certain formats used with time.strftime()), you will have to find a way to do it without using the standard library routine.
https://docs.python.org/3/library/locale.html#background-details-hints-tips-and-caveats

I think that just changing the regex to return the three elements and then doing a simply lookup on the first three characters of the month should do it:

astropy/astropy/utils/iers/iers.py

Line 876 in a68f456

_re_expires = re.compile(r'^#.*File expires on[:\s]+(\d+\s\w+\s\d+)\s*$')

saimn · 2020-11-25T02:20:18Z

astropy/utils/iers/iers.py

+                    day, month, year = match.groups()[0].split()
+                    month_nb = MONTH_ABBR.index(month[:3]) + 1
+                    expires = Time(f'{year}-{month_nb:02d}-{day}',
+                                   scale='tai', out_subfmt='date')


@mhvk - Something like this ? 🔝

saimn · 2020-11-25T02:20:50Z

About locale tests, the one in test_misc.py was skipped despite the de locale was installed, should be ok now. And the one in test_wcsprm.py was xfailed on master:

 ../../.tox/py37-test/lib/python3.7/site-packages/astropy/utils/tests/test_misc.py . [ 94%]
.s..s.  
../../.tox/py37-test/lib/python3.7/site-packages/astropy/wcs/tests/test_wcsprm.py . [ 97%]
........................................................................ [ 98%]
...........................x.....................................        [ 98%]

saimn · 2020-11-25T02:32:28Z

With the PR:

../../.tox/py37-test/lib/python3.7/site-packages/astropy/utils/tests/test_misc.py . [ 94%]
.s....                                                                   [ 94%]
../../.tox/py37-test/lib/python3.7/site-packages/astropy/io/ascii/tests/test_read.py . [ 22%]
........................................................................ [ 22%]
.......................................................................  [ 23%]
../../.tox/py37-test/lib/python3.7/site-packages/astropy/utils/iers/tests/test_leap_second.py . [ 92%]
.........ss..s...s.............                                          [ 93%]

mhvk

This looks good! Does need a changelog entry... And maybe squash the commits?

- A locale test in test_wcsprm already use fr_FR and was not run - The tests in test_read and test_misc were not run on the CI, probably because of the missing encoding So instead of installing two locales in the CI environment, migrate all test to use fr_FR.

saimn · 2020-11-25T22:51:15Z

@mhvk - Done!

mhvk

Great, merging...

Fix leap second update when using a non english locale

saimn added the Bug label Nov 19, 2020

saimn added this to the v4.0.4 milestone Nov 19, 2020

saimn requested a review from mhvk November 19, 2020 22:39

astropy-bot bot added utils utils.iers labels Nov 19, 2020

github-actions bot removed the utils label Nov 19, 2020

mhvk reviewed Nov 20, 2020

View reviewed changes

saimn force-pushed the leap-second-locale branch from 12d58c9 to 5e32975 Compare November 25, 2020 02:01

saimn commented Nov 25, 2020

View reviewed changes

mhvk reviewed Nov 25, 2020

View reviewed changes

bsipocz modified the milestones: v4.0.4, v4.0.5 Nov 25, 2020

saimn added 2 commits November 25, 2020 19:48

Fix leap second update when using a non english locale

5ea5499

saimn force-pushed the leap-second-locale branch from dcaf0ec to 15bd061 Compare November 25, 2020 22:50

mhvk approved these changes Nov 26, 2020

View reviewed changes

mhvk merged commit b40aa18 into astropy:master Nov 26, 2020

saimn deleted the leap-second-locale branch November 26, 2020 01:33

saimn mentioned this pull request Feb 11, 2021

test_precision errors if LANG is set to de_DE.UTF-8 #11321

Closed

astrofrog pushed a commit that referenced this pull request Mar 15, 2021

Merge pull request #11062 from saimn/leap-second-locale

903cd39

Fix leap second update when using a non english locale

eteq pushed a commit that referenced this pull request Mar 23, 2021

Merge pull request #11062 from saimn/leap-second-locale

8a2a02f

Fix leap second update when using a non english locale

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix leap second update when using a non english locale #11062

Fix leap second update when using a non english locale #11062

saimn commented Nov 19, 2020

pllim commented Nov 19, 2020

saimn commented Nov 19, 2020 •

edited

mhvk left a comment

mhvk Nov 20, 2020

saimn Nov 24, 2020

mhvk Nov 24, 2020

mhvk Nov 20, 2020

saimn Nov 24, 2020

mhvk Nov 24, 2020

saimn Nov 25, 2020

mhvk Nov 25, 2020

saimn commented Nov 25, 2020

saimn commented Nov 25, 2020

mhvk left a comment

saimn commented Nov 25, 2020

mhvk left a comment

	def test_locale():
	try:
	with _set_locale('fr_FR'):
	header = get_pkg_data_contents('data/locale.hdr',
	encoding='binary')
	with pytest.warns(FITSFixedWarning):
	w = _wcs.Wcsprm(header)
	assert re.search("[0-9]+,[0-9]*", w.to_header()) is None
	except locale.Error:
	pytest.xfail(
	"Can't set to 'fr_FR' locale, perhaps because it is not installed "
	"on this system")

Fix leap second update when using a non english locale #11062

Fix leap second update when using a non english locale #11062

Conversation

saimn commented Nov 19, 2020

pllim commented Nov 19, 2020

saimn commented Nov 19, 2020 • edited

mhvk left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

saimn commented Nov 25, 2020

saimn commented Nov 25, 2020

mhvk left a comment

Choose a reason for hiding this comment

saimn commented Nov 25, 2020

mhvk left a comment

Choose a reason for hiding this comment

saimn commented Nov 19, 2020 •

edited