Initial package content and GitHub Actions workflows by astrofrog · Pull Request #1 · astropy/astropy-iers-data

astrofrog · 2023-05-25T11:45:28Z

The main branch contains a basic package layout with no data etc. This PR adds the IERS A, B, and Leap Second files to the package and sets up the basic 'API', which consists (currently) of variables at the top level of the package which are as follows:

__all__ = [
    "IERS_A_FILE",
    "IERS_A_URL",
    "IERS_A_URL_MIRROR",
    "IERS_A_README",
    "IERS_B_FILE",
    "IERS_B_URL",
    "IERS_B_README",
    "IERS_LEAP_SECOND_FILE",
    "IERS_LEAP_SECOND_URL",
    "IETF_LEAP_SECOND_URL"
]

This is the only public API of the package. I decided to include the URLs in this package since it then makes it easier to update things if URLs break.

I have set up tox so that it will run the latest stable astropy test suite against the latest developer version of this package - I originally was only testing astropy.utils.iers but realized we should check also that coordinates, time, etc don't break. This currently tests a branch of astropy in my fork, but we can update that to astropy main once changes are merged in astropy core, and then to a stable version of astropy once it is in a released version.

I have also added four workflows:

One that will run the astropy test suite on different platforms/Python versions on commits and pull requests, as well as tags (that will run in this PR)
One that will regularly open pull requests with updates to the files, and let us decide when to merge updates. In future we could potentially also just automate this step and push directly to main, but I thought having some oversight initially would be good
One that will automatically create and push a calendar version tag (e.g. v2023.05.25) when there is a commit to main, which will happen when the aforementioned pull requests are merged
One that will publish the package on PyPI when a tag is created (only if the test suite passes)

With this, the actual maintainer workflow is to review and merge regular (currently weekly but we could change) pull requests to update the data files. The rest is fully automated.

In future, we could also potentially consider having some basic parsing functions in this package so that astropy core doesn't need to care about the details if the format of the tables changes - it could instead always access e.g. a Numpy structured array. But for now maybe best to keep things simple.

Open questions:

Is there a reason that IETF_LEAP_SECOND_URL can't be called IERS_LEAP_SECOND_URL_MIRROR?
Do we need to include ReadMe.eopc04_IAU2000 ?
One slightly annoying limitation of GitHub Actions is that the auto tagging workflow can't use the default GitHub Actions token, because if it does, the resulting tag doesn't cause the publish workflow to be run (a GA workflow with the default token can't trigger another workflow to run). So we need to use a personal access token. I can add one for my user to the repo, but personal tokens can only last a year at most, so we need to have a good way to update this. Alternatively I wonder if there is a way we can perhaps create the tag and run the publish workflow in the same run, but this needs to be investigated (maybe @ConorMacBride has some ideas? basically this is a general problem with fully automated releases - maybe there is a better way to do this than the workflow setup I have here?)

Once this is merged and there is a release of the package, I can move on to do the PR for astropy core.

nstarman · 2023-05-25T14:11:46Z

Can we use PEP 621 here? setup.cfg -> pyproject.toml

Yes - are there any easy migration tools? Or feel free to push a commit here if you are familiar with the new pyproject.toml format for this metadata.

Done, though the email I used is not valid. Does Astropy have an email?

nstarman · 2023-05-25T14:14:52Z

This is fine, but it would be more secure if we checked cryptographic hashes, e.g. with pooch

Sounds good - I'm not sure how to do that though, so any contributions there welcome!

taldcroft · 2023-05-25T16:22:23Z

@astrofrog - no comment on all the infrastructure stuff, its way beyond me.

About the two questions:

Is there a reason that IETF_LEAP_SECOND_URL can't be called IERS_LEAP_SECOND_URL_MIRROR?
Do we need to include ReadMe.eopc04_IAU2000 ?

Given this very substantial refactoring, it seems like the safest/easiest thing to do is stick with the original for now. After the dust has settled we can talk about clean-up. There is this (where it is not quite 2025...):

def test_IERS_B_old_style_excerpt():
    """Check that the instructions given in `IERS_B.read` actually work."""
    # If this test is changed, be sure to also adjust the instructions.
    #
    # TODO: this test and the note can probably be removed after
    # enough time has passed that old-style IERS_B files are simply
    # not around any more, say in 2025.  If so, also remove the excerpt
    # and the ReadMe.eopc04_IAU2000 file.
    old_style_file = get_pkg_data_filename(
        os.path.join("data", "iers_b_old_style_excerpt")
    )
    excerpt = iers.IERS_B.read(
        old_style_file,
        readme=get_pkg_data_filename(
            "data/ReadMe.eopc04_IAU2000", package="astropy.utils.iers"
        ),
        data_start=14,
    )

taldcroft · 2023-05-25T16:27:56Z

Since the IERS_B class documentation specifically shows using get_pkg_data_filename(..., package='astropy.utils.iers'), I wonder if there should be some special-case in get_pkg_data_filename to redirect package='astropy.utils.iers' into the new astropy-iers-data data space.

That would be needed to keep the existing astropy tests passing, but maybe that's just too much?

nstarman · 2023-05-25T20:37:30Z

Can we use a src layout?

astrofrog · 2023-05-25T20:39:37Z

Can we use a src layout?

I would say no for now for consistency with other packages in the astropy org.

nstarman · 2023-05-25T20:48:50Z

Unfortunately Astropy doesn't seem happy with Path-like objects, so the paths need to be strings for now. It would be good to fix that Astropy-side.

saimn · 2023-05-26T09:30:54Z


-build-backend = 'setuptools.build_meta'
+[project]
+  name = "astropy_iers_data"


Package name should probably use dashes instead of underscores ?

saimn · 2023-05-26T09:32:08Z

+  license = {file = "LICENSE.rst"}
+  keywords = ["astropy", "data", "IERS"]
+  authors = [
+    {name = "Astropy Developers", email = "[email protected]"}


Other packages use [email protected].

saimn · 2023-05-26T09:32:44Z

+[project.urls]
+  homepage = "https://github.com/astropy/astropy-iers-data"
+  repository = "https://github.com/astropy/astropy-iers-data"
+  documentation = "https://github.com/astropy/astropy-iers-data"


No need to put 3 times the same URL ?

Debateably. There's no standard, but some common ones (which I've put here). We can refine the links somewhat.

e.g.

repsository = "https://github.com/astropy/astropy-iers-data/src/astropy_iers_data" documentation = "https://github.com/astropy/astropy-iers-data/README.rst"

I'll update documentation to point to the README and remove homepage as there isn't really one as such (that is not the repository)

saimn · 2023-05-26T09:34:57Z

 commands =
    pip freeze
-    pytest --pyargs astropy_iers_data {toxinidir}/docs --cov astropy_iers_data --cov-config={toxinidir}/setup.cfg {posargs}
+    pytest --pyargs astropy {posargs}


Can we run tests only for some subpackages ( I guess utils.iers and coordinates) ?

I thought of this but we also have to include astropy.time and then I was thinking well why not just run it all as in case by chance we ever add usage of IERS elsewhere in the core package? It will only be running weekly so maybe not a major CI usage?

astrofrog · 2023-05-30T06:45:00Z

@taldcroft - I have been thinking about your comment here:

Given this very substantial refactoring, it seems like the safest/easiest thing to do is stick with the original for now

Because of the nature of the package, it won't be a trivial matter to update/cleanup API once this package starts getting used. If a user installs astropy 6.0 in 2026 (not completely impossible), unless we do anything as mentioned below the latest astropy-iers-data will be pulled in and so we will need to make sure that that version is still compatible with astropy 6.0.

This had led me to think that perhaps in fact semantic version would in fact be better than calendar versioning, in case we do ever need to update the API. Then, astropy core could include e.g. astropy-iers-data>1.2;<2 as a requirement if 1.2 where we basically know that astropy core won't work in future with a hypothetical 2.x release series in future since a major version bump would by definition mean an API breakage. We could then bump the major version of astropy-iers-data once the most recent version of astropy core using incompatible 1.x versions is quite old (as in years) and for those users recommend the manual download route. It would be good to keep major releases exceptional though, this would just be a safety mechanism.

We could use a hybrid semantic/calendar version where we use major.year.month.day, e.g. 1.2023.05.25 so that we still have date info but keep the first digit for actual API breakage?

In any case, this is all possible but my point is that API cleanup/breakage is not something to be taken lightly, but at the same time I understand maybe it will complicate this PR to discuss it here and would be easier to discuss each change in a dedicated PR. So my suggestion would be to try and proceed with this PR but to try and refine/clean up the API before we start using the package in astropy core for good.

mhvk · 2023-05-30T11:14:08Z

I think there is a lot to be said for making the first commit as easy to understand as possible - i.e., close to just a copy of what what in astropy - and then have further PRs to (slightly) adjust the API before a first official release.

taldcroft · 2023-06-01T10:19:40Z

@astrofrog - I agree with @mhvk on getting this first PR merged with minimal API changes and then go from there.

On the point of IETF_LEAP_SECOND_URL, doing this GitHub search IETF_LEAP_SECOND_URL AND NOT (path:iers.py OR path:test_leap_second.py) shows exactly one repo that is using that name. My general inclination is to accept a slightly imperfect situation (inconsistently named parameter) over potentially breaking downstream code, but I don't feel that strongly here and could go either way.

And most likely removing that old IERS-A file is fine.

Co-authored-by: Conor MacBride <[email protected]>

astrofrog · 2023-06-01T10:38:50Z

Thanks everyone for the discussion! Once the CI passes, I will just merge and we can discuss further improvements in follow-up issues/PRs.

astrofrog mentioned this pull request May 25, 2023

Split out IERS Earth Rotation and Leap Second files into separate package astropy/astropy#14819

Merged

5 of 6 tasks

astrofrog requested review from mhvk, pllim and taldcroft May 25, 2023 11:56

nstarman reviewed May 25, 2023

View reviewed changes

astrofrog added 8 commits May 25, 2023 15:47

Initial package content

551bdfe

Added update workflow for GitHub Actions

9530da2

Updated tox configuration and added main CI workflow

f5050e9

Add automatic tagging on commits to main

f74a1c6

Cleanup

420bebe

Shorten version

bd607a7

Added note about API

b78ef39

Run full astropy test suite

5c690f0

astrofrog force-pushed the initial-package branch from 0ede7b7 to 2905460 Compare May 25, 2023 14:47

nstarman force-pushed the initial-package branch from 9e45a16 to ce7a66b Compare May 25, 2023 20:49

saimn reviewed May 26, 2023

View reviewed changes

ConorMacBride reviewed May 28, 2023

View reviewed changes

Comment thread .github/workflows/main.yml Resolved

astrofrog mentioned this pull request Jun 1, 2023

Support pathlib.Path objects wherever possible astropy/astropy#14893

Open

astrofrog and others added 4 commits June 1, 2023 11:30

Use pathlib, but call str() until Astropy can work with pathlib.Path

61b3505

Remove unused files

d8520e4

Added note about temporary git clone URL

62cb6e1

Replace setup.cfg with pyproject

d646e59

astrofrog and others added 4 commits June 1, 2023 11:30

Combine tagging and publishing workflow

129e1c4

Co-authored-by: Conor MacBride <[email protected]>

Updates to metadata

476c1f8

Remove unecessary docs/ folder, and update MANIFEST.in

c110a61

Further tidying up of GitHub Actions configuration

41d8e72

astrofrog force-pushed the initial-package branch from c6ec4fc to 41d8e72 Compare June 1, 2023 10:31

Speed up testing

f03e66a

astrofrog merged commit 2e82b23 into astropy:main Jun 1, 2023

neutrinoceros mentioned this pull request Mar 18, 2026

CLN: clean up an unmatched parentheis in a description field #61

Closed

Conversation

astrofrog commented May 25, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

taldcroft commented May 25, 2023

Uh oh!

taldcroft commented May 25, 2023

Uh oh!

nstarman commented May 25, 2023

Uh oh!

astrofrog commented May 25, 2023

Uh oh!

nstarman commented May 25, 2023

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

astrofrog commented May 30, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

mhvk commented May 30, 2023

Uh oh!

taldcroft commented Jun 1, 2023

Uh oh!

astrofrog commented Jun 1, 2023

Uh oh!

Reviewers

Uh oh!

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

astrofrog commented May 25, 2023 •

edited

Loading

astrofrog commented May 30, 2023 •

edited

Loading