Skip to content

Added workflow and script to regularly update the minimum required version of astropy-iers-data - #15051

Merged
pllim merged 4 commits into
astropy:mainfrom
astrofrog:auto-update-astropy-iers-data-pin
Aug 1, 2023
Merged

pllim merged 4 commits into
astropy:mainfrom
astrofrog:auto-update-astropy-iers-data-pin

Conversation

@astrofrog

Copy link
Copy Markdown
Member

Also set the initial pin. Currently I have set this to run monthly as that is probably enough, I don't think we need to actually update it weekly as soon as there is an astropy-iers-data release - but happy to adjust.

Fixes #15046
Fixes #14951

Example PR in my fork: astrofrog#125

@github-actions

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Astropy! 🌌 This checklist is meant to remind the package maintainers who will review this pull request of some common things to look for.

  • Do the proposed changes actually accomplish desired goals?
  • Do the proposed changes follow the Astropy coding guidelines?
  • Are tests added/updated as required? If so, do they follow the Astropy testing guidelines?
  • Are docs added/updated as required? If so, do they follow the Astropy documentation guidelines?
  • Is rebase and/or squash necessary? If so, please provide the author with appropriate instructions. Also see "When to rebase and squash commits".
  • Did the CI pass? If no, are the failures related? If you need to run daily and weekly cron jobs as part of the PR, please apply the "Extra CI" label. Codestyle issues can be fixed by the bot.
  • Is a change log needed? If yes, did the change log check pass? If no, add the "no-changelog-entry-needed" label. If this is a manual backport, use the "skip-changelog-checks" label unless special changelog handling is necessary.
  • Is this a big PR that makes a "What's new?" entry worthwhile and if so, is (1) a "what's new" entry included in this PR and (2) the "whatsnew-needed" label applied?
  • Is a milestone set? Milestone must be set but we cannot check for it on Actions; do not let the green checkmark fool you.
  • At the time of adding the milestone, if the milestone set requires a backport to release branch(es), apply the appropriate "backport-X.Y.x" label(s) before merge.

@astrofrog

Copy link
Copy Markdown
Member Author

@nstarman - how do I tell ruff to ignore the following error in this case?

.github/workflows/update_astropy_iers_data_pin.py:1:1: INP001 File `.github/workflows/update_astropy_iers_data_pin.py` is part of an implicit namespace package. Add an `__init__.py`.

@astrofrog
astrofrog requested a review from pllim July 10, 2023 11:11
@nstarman

Copy link
Copy Markdown
Member

@nstarman - how do I tell ruff to ignore the following error in this case?


.github/workflows/update_astropy_iers_data_pin.py:1:1: INP001 File `.github/workflows/update_astropy_iers_data_pin.py` is part of an implicit namespace package. Add an `__init__.py`.

We should probably add a per-file-ignore for the whole workflows folder. In the pyproject.toml [tool.ruff.per-file-ignores] section add a glob for the folder.

@astrofrog

Copy link
Copy Markdown
Member Author

@nstarman - I tried adding this in 09bf460 but it doesn't seem to be having any effect. Any idea what might be going on?

@nstarman

Copy link
Copy Markdown
Member

We aren't (yet) using cascading per-file-ignores, so the .ruff.toml file takes precedence. The config in pyproject.toml needs to be moved there. I think that's what's going on.

fi
- name: Create Pull Request
uses: peter-evans/create-pull-request@v5
with:

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.

I see above that you only make a commit if there is an actual change. But if there is no change, will there still be a PR created?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If there are no commits different compared to the PR base, no PR is opened

@astrofrog
astrofrog force-pushed the auto-update-astropy-iers-data-pin branch from 09bf460 to 9d3c21d Compare August 1, 2023 09:41
@astrofrog
astrofrog force-pushed the auto-update-astropy-iers-data-pin branch from 9d3c21d to 632a07a Compare August 1, 2023 09:49
@astrofrog

Copy link
Copy Markdown
Member Author

I've now rebased this and hopefully the CI will pass - if it does and we can merge it in today, then the cron job should run tomorrow.


last_version = metadata["info"]["version"]

with open("setup.cfg") as f:

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.

If we ever get rid of setup.* (e.g., #14361 would get rid of setup.py and someone had mentioned that we should move everything to pyproject.toml but I cannot find the issue), this will need to be updated.

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 believe @WilliamJamieson and I were the primary advocates for the pyproject.toml everything.
Python 3.11 has a batteries-included TOML library, so we can use that!

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

It would be easy to switch this over to pyproject.toml once we need to

Comment thread setup.cfg
numpy>=1.22
pyerfa>=2.0
astropy-iers-data
astropy-iers-data>=0.2023.7.10.0.36.50

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.

Why not the latest release from 2 days ago?

Suggested change
astropy-iers-data>=0.2023.7.10.0.36.50
astropy-iers-data>=0.2023.07.31.00.32.45

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

If we leave this as a slightly older version it will be a good test that the cron job works tomorrow?

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.

OKay but you are still missing a leading zero. .7 should be .07

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

Also leading zeros shouldn't be needed in version numbers for 2023.12 to evaluate as more recent than 2023.7 - I'm not opposed to adding leading zeros if people want but they aren't needed

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.

Hmm ok so the PyPI version is inconsistent with GitHub tags.

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

Let's merge then and see if cron job works. Thanks!

@pllim
pllim merged commit ead1fd2 into astropy:main Aug 1, 2023
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.

Automate updating of minimum version pin of astropy-iers-data in astropy core Set minimum pin for astropy-iers-data

4 participants