TST: Disable IERS download for testing #10311

pllim · 2020-05-05T16:21:18Z

Description

This pull request is to follow-up on the idea in #10304 (comment) that we can disable auto-downloading of IERS data completely for testing purpose.

The proof-of-concept part of this is in docs/coordinates/satellites.rst in that this fixes #10255 albeit slight changes in expected results. (This builds upon #10304 to disable of_site download.)

~~This also moves stuff from astropy/conftest.py up to root level conftest.py, as stated in #10310.~~

~~I took advantage of a set but unused builtins._pytest_running variable I saw in astropy/conftest.py.~~ If people are interested to disable it outside of testing environment, say, for computing clusters or multiprocessing, we might need to resort to setting system environment variable instead though I am not sure if even that would solve the problem for everyone because it solves the download problem but not if you still need to download something but has race condition in caching.

TODO

Are we really doing this?
Fix failing tests -- most of them will warn about something is out of range and need to be handled accordingly
Not sure how to identify the rest of the tests that no longer need really need remote data that are not failing. (Battle for another day.)

Bonus:

Minor PEP 8 fixes.
Address some of Ideally should avoid hardcoded file path in tests #10344 where I see them.
Use pathlib to handle file URIs.
Attempt to fix flaky test_positions_skyfield failing CI.

mhvk · 2020-05-05T17:19:50Z

Nice find! But would it be possible to go one step further in the pytest configuration in conftest.py, and simply set iers.confi.auto_download=False there (and reset at the end)? If that works, we do not have to touch iers.py at all! Even better if it is possible to only do this for remote_data!='any'...

pllim · 2020-05-05T17:21:48Z

Setting conf didn't work in astropy/conftest.py, and then I got distracted with builtins. Let me try it again but at root level conftest.py.

mhvk · 2020-05-05T17:24:49Z

I hope this works. Yet another option might be to have a doctest marking +OFFLINE, which disables temporarily disables any internet connection. But to be honest that seems even trickier.

pllim · 2020-05-05T18:48:31Z

Even better if it is possible to only do this for remote_data!='any'

That seems like a lot more work, as then the setting needs to be set at a per-test level.

mhvk · 2020-05-05T18:58:30Z

Ah, yes, and the remote_data should not really be about testing IERS download anyway.

This does look nice! But it seems the test failures are real: I guess for some tests we need to explicitly enable downloading now. (Can look later.)

pllim · 2020-05-05T19:01:02Z

test failures are real

Yes. I added that as part of my todo in the original comment above. But before I spend the time fixing the tests, I need to know if we're really doing this. 😄

mhvk · 2020-05-05T19:46:06Z

Fair enough to fix only once we all agree! FWIW, I'm in favour. Only tests that explicitly check for the updating should be allowed to try to get new versions.

Perhaps the main question is how to do it. E.g., an alternative might be to use a configuration file in tests that turns downloading off. On balance, though, I think that what you have is better. Anyway, let's also bring in @astrofrog and @bsipocz...

mhvk · 2020-06-03T00:54:17Z

Looking at this again, I still like the solution. But I do worry about deleting astropy/conftest.py - I think that is needed for tests run from within python (via astropy.test).

astrofrog · 2020-06-03T11:42:44Z

Looking at this again, I still like the solution. But I do worry about deleting astropy/conftest.py - I think that is needed for tests run from within python (via astropy.test).

Indeed if we want astropy.test() to work we can't move the conftest.py file out of the package unfortunately.

pllim · 2020-06-03T16:26:52Z

Okay, I'll reduce the scope when I get the chance. Thanks for the review!

pllim · 2020-06-04T18:28:52Z

If this fails with the settings in astropy/conftest.py, which I think it did based on what Past Self said in #10311 (comment) , will setting another pytest_configure/pytest_unconfigure in root level conftest.py without touching astropy/conftest.py the correct way to go?

pllim · 2020-06-04T18:59:04Z

Huh... The error I thought I would get didn't happen. I guess I'll fix the test failures and see what happens next.

pllim · 2020-06-05T02:38:25Z

Remote data job passed.

astrofrog

Seems reasonable to me from a testing perspective, but maintainers of the relevant sub-packages should review the test updates.

brandon-rhodes · 2020-06-05T14:20:38Z

astropy/coordinates/tests/test_solar_system.py

    # skyfield ephemeris
    try:
        planets = load('de421.bsp')
+        ts = load.timescale()


It might go against the intention here, but load.timescale(builtin=True) will suppress file download and use built-in copies of the leap second files instead.

I thought about that but I don't know what is really being tested here and the test is already marked as a remote data test. Maybe @adrn or @eteq can clarify.

This was there already, just moved up so that if, somehow, skyfield tries to access the internet, we'll fail on jobs where we do not allow that. I think it is OK.

Also: the tests here are for specific times in the past, so should be OK not to update the leap-second list!

@mhvk , did you mean that I should have used ts = load.timescale(builtin=True) and not try to ignore its timeout error in CI? Can you please clarify? If so, I can open a follow-up PR.

No, sorry, I misunderstood here - this remains a remote-date test and what you did is exactly the right thing!

mhvk

This looks great!

I must admit I'm also a bit confused that just changing astropy/conftest.py seems to solve not just the tests but also the docs, but, hey, not going to complain!

Could you open an issue for removing remote_date markers that were there just because of IERS downloads??

mhvk · 2020-06-05T18:10:27Z

I'm merging since I think I can remove at least one remote_data already in a PR I'm doing!

pllim · 2020-06-05T20:47:29Z

Issue opened at #10457 . Thanks, everyone!

mhvk · 2020-06-06T14:55:20Z

@pllim - I am confused: I tried over in #10337 to remove a remote_data check on a test that doesn't use any times in the far future, yet it failed miserably:
https://travis-ci.org/github/astropy/astropy/jobs/695168048

Now the build log is quite hard to parse, but if I do it locally I get a traceback which suggests the config change is not picked up:

../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/tests/test_shape_manipulation.py:61: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/earth.py:695: in get_gcrs_posvel
    gcrs_data = self.get_gcrs(obstime).data
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/earth.py:675: in get_gcrs
    return itrs.transform_to(GCRS(obstime=obstime))
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/baseframe.py:1220: in transform_to
    return trans(self, new_frame)
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/transformations.py:1392: in __call__
    curr_coord = t(curr_coord, curr_toframe)
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/transformations.py:928: in __call__
    reprwithoutdiff = supcall(from_diffless, toframe)
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/builtin_frames/intermediate_rotation_transforms.py:107: in itrs_to_cirs
    pmat = cirs_to_itrs_mat(itrs_coo.obstime)
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/builtin_frames/intermediate_rotation_transforms.py:51: in cirs_to_itrs_mat
    xp, yp = get_polar_motion(time)
../../.tox/test/lib/python3.8/site-packages/astropy/coordinates/builtin_frames/utils.py:41: in get_polar_motion
    iers_table = iers.earth_orientation_table.get()
../../.tox/test/lib/python3.8/site-packages/astropy/utils/state.py:40: in get
    return cls.validate(cls._value)
../../.tox/test/lib/python3.8/site-packages/astropy/utils/iers/iers.py:866: in validate
    value = IERS_Auto.open()
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

cls = <class 'astropy.utils.iers.iers.IERS_Auto'>

    @classmethod
    def open(cls):
        """If the configuration setting ``astropy.utils.iers.conf.auto_download``
        is set to True (default), then open a recent version of the IERS-A
        table with predictions for UT1-UTC and polar motion out to
        approximately one year from now.  If the available version of this file
        is older than ``astropy.utils.iers.conf.auto_max_age`` days old
        (or non-existent) then it will be downloaded over the network and cached.
    
        If the configuration setting ``astropy.utils.iers.conf.auto_download``
        is set to False then ``astropy.utils.iers.IERS()`` is returned.  This
        is normally the IERS-B table that is supplied with astropy.
    
        On the first call in a session, the table will be memoized (in the
        ``iers_table`` class attribute), and further calls to ``open`` will
        return this stored table.
    
        Returns
        -------
        `~astropy.table.QTable` instance with IERS (Earth rotation) data columns
    
        """
        if not conf.auto_download:
            cls.iers_table = IERS_B.open()
            return cls.iers_table
    
        all_urls = (conf.iers_auto_url, conf.iers_auto_url_mirror)
    
        if cls.iers_table is not None:
    
            # If the URL has changed, we need to redownload the file, so we
            # should ignore the internally cached version.
    
            if cls.iers_table.meta.get('data_url') in all_urls:
                return cls.iers_table
    
        try:
            filename = download_file(all_urls[0], sources=all_urls, cache=True)
        except Exception as err:
            # Issue a warning here, perhaps user is offline.  An exception
            # will be raised downstream when actually trying to interpolate
            # predictive values.
>           warn(AstropyWarning(
                f'failed to download {" and ".join(all_urls)}, '
                f'using local IERS-B: {err}'))
E           astropy.utils.exceptions.AstropyWarning: failed to download ftp://anonymous:mail%40astropy.org@gdc.cddis.eosdis.nasa.gov/pub/products/iers/finals2000A.all and https://datacenter.iers.org/data/9/finals2000A.all, using local IERS-B: <urlopen error Unable to open any source! Exceptions were {'ftp://anonymous:mail%40astropy.org@gdc.cddis.eosdis.nasa.gov/pub/products/iers/finals2000A.all': URLError("ftp error: OSError('An attempt was made to connect to the internet by a test that was not marked `remote_data`. The requested host was: 198.118.242.43')"), 'https://datacenter.iers.org/data/9/finals2000A.all': URLError(OSError('An attempt was made to connect to the internet by a test that was not marked `remote_data`. The requested host was: datacenter.iers.org'))}>

mhvk · 2020-06-06T14:57:35Z

p.s. For now, I've just put the remote_data back on #10337, but I remain very confused! Did we need to do the home conftest.py as well after all?

pllim · 2020-06-08T21:18:00Z

@mhvk , I am confused too. I have opened an investigation over at #10469.

astrofrog · 2021-08-17T10:02:03Z

This is needed in LTS so I have updated the milestone and backported it.

TST: Disable IERS download for testing

pllim added the Experimental label May 5, 2020

pllim requested review from eteq and mhvk May 5, 2020 16:21

pllim added utils.iers testing labels May 5, 2020

pllim force-pushed the tst-no-iers-download branch from 7f3fd24 to 3cf5386 Compare May 5, 2020 18:41

pllim added the no-changelog-entry-needed label May 5, 2020

mhvk mentioned this pull request May 5, 2020

EXP: EarthLocation context manager to force builtin sites #10313

Closed

pllim force-pushed the tst-no-iers-download branch from 3cf5386 to d230930 Compare June 4, 2020 18:26

pllim removed the Experimental label Jun 4, 2020

pllim added this to the v4.2 milestone Jun 4, 2020

pllim changed the title ~~EXP: Disable IERS download for testing~~ TST: Disable IERS download for testing Jun 4, 2020

pllim force-pushed the tst-no-iers-download branch from c8b5486 to 872b2ba Compare June 4, 2020 21:43

This comment has been minimized.

Sign in to view

pllim added 4 commits June 4, 2020 21:08

TST: Disable IERS download for testing.

ea7953b

Fix failures in initial test stage

d87e875

Not sure why this is different on my machine

9103b0b

Fix remote data tests

a622e7e

pllim force-pushed the tst-no-iers-download branch from 872b2ba to a622e7e Compare June 5, 2020 01:08

Fix more remote data failures

c328f8f

pllim added the Ready-for-final-review label Jun 5, 2020

astrofrog approved these changes Jun 5, 2020

View reviewed changes

brandon-rhodes reviewed Jun 5, 2020

View reviewed changes

mhvk approved these changes Jun 5, 2020

View reviewed changes

mhvk merged commit f9be92d into astropy:master Jun 5, 2020

pllim deleted the tst-no-iers-download branch June 5, 2020 20:35

pllim mentioned this pull request Jun 5, 2020

TST: Revisit tests marked as remote data that can now use offline IERS #10457

Open

pllim mentioned this pull request Jun 8, 2020

EXP: Run TestManipulation without remote data #10469

Closed

mhvk mentioned this pull request Jun 9, 2020

Remove unnecessary remote_data in (mostly) coordinates. #10474

Merged

astrofrog modified the milestones: v4.2, v4.0.6 Aug 17, 2021

astrofrog pushed a commit that referenced this pull request Aug 17, 2021

Merge pull request #10311 from pllim/tst-no-iers-download

bcf7572

TST: Disable IERS download for testing

Uh oh!

TST: Disable IERS download for testing #10311

TST: Disable IERS download for testing #10311

Uh oh!

Conversation

pllim commented May 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Uh oh!

mhvk commented May 5, 2020

Uh oh!

pllim commented May 5, 2020

Uh oh!

mhvk commented May 5, 2020

Uh oh!

pllim commented May 5, 2020

Uh oh!

mhvk commented May 5, 2020

Uh oh!

pllim commented May 5, 2020

Uh oh!

mhvk commented May 5, 2020

Uh oh!

mhvk commented Jun 3, 2020

Uh oh!

astrofrog commented Jun 3, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Jun 3, 2020

Uh oh!

pllim commented Jun 4, 2020

Uh oh!

pllim commented Jun 4, 2020

Uh oh!

This comment has been minimized.

pllim commented Jun 5, 2020

Uh oh!

astrofrog left a comment

Choose a reason for hiding this comment

Uh oh!

brandon-rhodes Jun 5, 2020

Choose a reason for hiding this comment

Uh oh!

pllim Jun 5, 2020

Choose a reason for hiding this comment

Uh oh!

mhvk Jun 5, 2020

Choose a reason for hiding this comment

Uh oh!

pllim Jun 5, 2020

Choose a reason for hiding this comment

Uh oh!

mhvk Jun 5, 2020

Choose a reason for hiding this comment

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk commented Jun 5, 2020

Uh oh!

pllim commented Jun 5, 2020

Uh oh!

mhvk commented Jun 6, 2020

Uh oh!

mhvk commented Jun 6, 2020

Uh oh!

pllim commented Jun 8, 2020

Uh oh!

astrofrog commented Aug 17, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

pllim commented May 5, 2020 •

edited

Loading

astrofrog commented Jun 3, 2020 •

edited

Loading