[7.0.0rc1] Mark some "future" test in coordinates as remote_data by olebole · Pull Request #17359 · astropy/astropy · GitHub
Skip to content

[7.0.0rc1] Mark some "future" test in coordinates as remote_data - #17359

Merged
pllim merged 1 commit into
mainfrom
coordinates-tests-remotedata
Nov 11, 2024
Merged

pllim merged 1 commit into
mainfrom
coordinates-tests-remotedata

Conversation

@olebole

@olebole olebole commented Nov 9, 2024

Copy link
Copy Markdown
Member

Description

These tests (seem to) try to pull remote date because of the not-covered time range, which should be marked as such. Otherwise, Build without remote access (like on Debian package builds) fails.

  • By checking this box, the PR author has requested that maintainers do NOT use the "Squash and Merge" button. Maintainers should respect this when possible; however, the final decision is at the discretion of the maintainer that merges the PR.

These tests (seem to) try to pull remote date because of the
not-covered time range, which should be marked as such. Otherwise,
Build without remote access (like on Debian package builds) fails.
@github-actions

github-actions Bot commented Nov 9, 2024

Copy link
Copy Markdown
Contributor

@neutrinoceros neutrinoceros left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

none of these tests are new. It's not immediately obvious to me why this is suddenly needed. Would it be possible to bisect the change so we have a better idea wether it was intended ?

@olebole

olebole commented Nov 10, 2024

Copy link
Copy Markdown
Member Author

Bisection is difficult in my workflow, as I take released tarballs as input for the Debian packaging (and need to adjust local patches to it). Here is the first failure:

______________________________ test_future_altaz _______________________________

    def test_future_altaz():
        […]
        with (
            ctx,
            pytest.warns(
                AstropyWarning,
                match="Tried to get polar motions for times after IERS data is valid.*",
            ),
        ):
>           SkyCoord(1 * u.deg, 2 * u.deg).transform_to(AltAz(location=location, obstime=t))

astropy/coordinates/tests/test_iau_fullstack.py:212: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 
astropy/coordinates/sky_coordinate.py:555: in transform_to
    new_coord = trans(self.frame, generic_frame)
astropy/coordinates/transformations/composite.py:113: in __call__
    curr_coord = t(curr_coord, curr_toframe)
astropy/coordinates/transformations/function.py:175: in __call__
    return supcall(fromcoord, toframe)
astropy/coordinates/builtin_frames/icrs_observed_transforms.py:35: in icrs_to_observed
    astrom = erfa_astrom.get().apco(observed_frame)
astropy/coordinates/erfa_astrom.py:57: in apco
    era = erfa.era00(*get_jd12(obstime, "ut1"))
astropy/coordinates/builtin_frames/utils.py:115: in get_jd12
    newtime = getattr(time, scale)
astropy/time/core.py:1769: in __getattr__
    tm._set_scale(attr)
astropy/time/core.py:828: in _set_scale
    args.append(get_dt(jd1, jd2))
astropy/time/core.py:2558: in _get_delta_ut1_utc
    delta = iers_table.ut1_utc(jd1, jd2)
astropy/utils/iers/iers.py:346: in ut1_utc
    return self._interpolate(
astropy/utils/iers/iers.py:500: in _interpolate
    self._check_interpolate_indices(i1, i, np.max(mjd, initial=50000))
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

self = <IERS_Auto length=19192>
 year month  day    MJD   PolPMFlag_A ... PolPMFlag dX_2000A dY_2000A NutFlag
               ...P ...         P      nan      nan       0
   25     7    19 60875.0           P ...         P      nan      nan       0
indices_orig = array([19191]), indices_clipped = array([19192])
max_input_mjd = 238303.0

    def _check_interpolate_indices(self, indices_orig, indices_clipped, max_input_mjd):
        """…"""
        predictive_mjd = self.meta["predictive_mjd"]
    
        # See explanation in _refresh_table_as_needed for these conditions
        auto_max_age = _none_to_float(conf.auto_max_age)
        if (
            max_input_mjd > predictive_mjd
            and self.time_now.mjd - predictive_mjd > auto_max_age
        ):
>           raise ValueError(INTERPOLATE_ERROR.format(auto_max_age))
E           ValueError: interpolating from IERS_Auto using predictive values that are more
E           than 30.0 days old.
E           
E           Normally you should not see this error […]
E           
E             from astropy.utils.iers import conf
E             conf.auto_max_age = None

astropy/utils/iers/iers.py:870: ValueError

During handling of the above exception, another exception occurred:

    def test_future_altaz():
        […]
        if PYTEST_LT_8_0:
            ctx = nullcontext()
        else:
            ctx = pytest.warns(erfa.core.ErfaWarning)
        with (
>           ctx,
            pytest.warns(
                AstropyWarning,
                match="Tried to get polar motions for times after IERS data is valid.*",
            ),
        ):
E       astropy.utils.exceptions.AstropyWarning: failed to download https://datacenter.iers.org/data/9/finals2000A.all and https://maia.usno.navy.mil/ser7/finals2000A.all: <urlopen error Unable to open any source! Exceptions were {'https://datacenter.iers.org/data/9/finals2000A.all': URLError(ConnectionRefusedError(111, 'Connection refused')), 'https://maia.usno.navy.mil/ser7/finals2000A.all': URLError(ConnectionRefusedError(111, 'Connection refused'))}>.
E       A coordinate or time-related calculation might be compromised or fail because the dates are not covered by the available IERS file.  See the "IERS data access" section of the astropy documentation for additional information on working offline.

astropy/coordinates/tests/test_iau_fullstack.py:206: AstropyWarning

and this looks to me that it tries to download the data because they are too old. 6.1.5 passes here. Indeed we use a rather old version of astropy-iers-data (0.2024.07.22.00.34.13).

@neutrinoceros

Copy link
Copy Markdown
Contributor

I see. Indeed I don't think we run tests against old versions of astropy-iers-data anywhere (and in fact require the newest version in testing), so it makes sense that we didn't catch this in CI. Do you have a sense why this wasn't seen for previous releases ?

@olebole

olebole commented Nov 10, 2024

Copy link
Copy Markdown
Member Author

I don't know. I have run the tests for 6.1.5 at almost the same time and didn't observe a failure. One difference may be that in 6.1.5 one additional warning was catched,

ctx2 = pytest.warns(AstropyWarning, match=".*times are outside of range.*")

Reverting to this didn't resolve it, but produced a failure that the specified warning was not raised. I guess the problem is that the IERS_Auto logic changed and this change was not correctly handled in #16187: In 6.1.5, it just raised that warning, but now it instead tried to download an actual file (which fails in my case). Maybe @ayshih could help here.

In Debian we need to have all dependencies resolved locally (i.e. no remote connections) and we want to have the packages buildable (incl. tests) during the lifetime of a distribution release; so I need to switch off tests that may become dependent from remote data after some time.

@pllim pllim left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Seems fine for now. I also saw similar behavior when moving coordinates examples from Example Gallery into proper docs. We have an issue somewhere to force true "offline" mode in some of these tests but that is a battle for another day. Thanks!

@pllim

pllim commented Nov 11, 2024

Copy link
Copy Markdown
Member

p.s. For future reference, please open PR from your fork. Thanks!

@pllim
pllim merged commit 5b1014b into main Nov 11, 2024
@pllim
pllim deleted the coordinates-tests-remotedata branch November 11, 2024 14:29
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Nov 11, 2024
@olebole

olebole commented Nov 11, 2024

Copy link
Copy Markdown
Member Author

p.s. For future reference, please open PR from your fork. Thanks!

Oh, yes, sorry, I didn't remember that I have write access :-) I will check this before my next PR.

pllim added a commit that referenced this pull request Nov 11, 2024
…359-on-v7.0.x

Backport PR #17359 on branch v7.0.x ([7.0.0rc1] Mark some "future" test in coordinates as remote_data)
@eerovaher

Copy link
Copy Markdown
Member

p.s. For future reference, please open PR from your fork. Thanks!

Oh, yes, sorry, I didn't remember that I have write access :-) I will check this before my next PR.

It is possible to configure Git so that even when you (accidentally) try push to the main astropy repository then you still only push to your fork on GitHub.

@ayshih

ayshih commented Nov 13, 2024

Copy link
Copy Markdown
Contributor

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants