Allow more tests to run without latest IERS data by eerovaher · Pull Request #17377 · astropy/astropy · GitHub
Skip to content

Allow more tests to run without latest IERS data - #17377

Merged
pllim merged 1 commit into
astropy:mainfrom
eerovaher:iers-future-times
Nov 12, 2024
Merged

pllim merged 1 commit into
astropy:mainfrom
eerovaher:iers-future-times

Conversation

@eerovaher

Copy link
Copy Markdown
Member

Description

In #17359 a few tests were marked as remote data tests because they tried to fetch IERS data over the Internet if the bundled data was too old. However, that change disables the tests in question entirely in normal test runs. The allowed age of the IERS data is configurable and the astropy configuration system has simple ways of modifying the configuration temporarily only for some block of code. It is therefore very simple to allow the tests to run even without Internet access.

To verify that the patch here works you must have an older version of astropy-iers-data installed so that the tests fail with some commit that predates #17359 (e.g. 59f1ada). You can then see that 382bb84 does indeed prevent test failures, but at the cost of skipping the tests entirely, and that this patch allows the tests to run successfully.

  • 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.

In 382bb84 (Mark some "future" test in
coordinates as remote_data) a few tests were marked as remote data tests
because they tried to fetch IERS data over the Internet if the bundled
data was too old. However, that change disables the tests in question
entirely in normal test runs. By applying a temporary configuration
change, it is possible to only prevent Internet access while still
allowing the tests to run.
@github-actions

Copy link
Copy Markdown
Contributor

@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.

LGTM thanks! But I'll give others a chance to review as well. Might as well backport.

@pllim pllim modified the milestones: v7.1.0, v7.0.0 Nov 11, 2024

@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.

Thanks for those clear reviewing guidelines !

@olebole olebole 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.

I can confirm that this works well for my use case (using iers-data 0.2024.07.22.00.34.13), and indeed this is probably a better solution for the problem than #17359.

@neutrinoceros

Copy link
Copy Markdown
Contributor

@pllim
pllim disabled auto-merge November 12, 2024 14:48
@pllim
pllim merged commit a3f20ce into astropy:main Nov 12, 2024
meeseeksmachine pushed a commit to meeseeksmachine/astropy that referenced this pull request Nov 12, 2024
@eerovaher
eerovaher deleted the iers-future-times branch November 12, 2024 15:02
pllim added a commit that referenced this pull request Nov 12, 2024
…377-on-v7.0.x

Backport PR #17377 on branch v7.0.x (Allow more tests to run without latest IERS data)
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.

4 participants