Dropping support for python 3.7

[bsipocz]

EDIT: This had to be broke up into 2 separate PRs to be able to add two changelog entries. See #11935 for dropping numpy

This is in agreement with APE18 and the email thread: https://groups.google.com/g/astropy-dev/c/Z9TPHm8DSmo/m/TgGT6BwFBAAJ?pli=1

Note: the changelog entry is not right, we will need two separate lines, both having the PR number at the end of the line when rendered.

I also expect some of the CI builds to fail, as I may not get the image names right at the first try.

• Restart CI after Debian updates upstream w.r.t. wcslib 7.6. Should happen by 2021-09-01.

Checklist for package maintainer(s)

This checklist is meant to remind the package maintainer(s) who will review this pull request of some common things to look for. This list is not exhaustive.

• 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.
• Is a change log needed? If yes, did the change log check pass? If no, add the no-changelog-entry-needed label.
• Is a milestone set? Milestone must be set but astropy-bot check might be missing; 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.

[nstarman]

Wooooh! Walrus operator!

               ___
            .-9 9 `\
          =(:(::)=  ;
            ||||     \
            ||||      `-.
           ,\|\|         `,
          /                \
         ;                  `'---.,
         |                         `\
         ;                     /     |
         \                    |      /
  jgs     )           \  __,.--\    /
       .-' \,..._\     \`   .-'  .-'
      `-=``      `:    |   /-/-/`
                   `.__/

[pllim]

Re: walrus operator -- Apparently not everyone was excited about it. See #9606

[pllim]

@bsipocz , thanks! I'll review once we decide whether to merge this (back) with #11935 or not rebase is done.

[mhvk]

The walrus will just come in slowly, I suspect, as people make commits that use it. Not quite like f-strings, where it is (relatively) easy to search for relevant places (and which were a really nice addition indeed!).

[bsipocz]

maybe switch directly to py39 instead of 38?

[pllim]

@astrofrog pointed to https://docs.astropy.org/en/latest/development/testguide.html#running-image-tests but someone with server access still needs to generate and upload the new image(s).

[pllim]

For the record, CircleCI stuff has been resolved over at #12032 . 😸

[mhvk]

Just had a look through the changes, and they all look good to me - indeed, rather little is affected. To me, it would seem to make sense to have the matplotlib images be built on 3.9 rather than 3.8, to avoid churn when the minimum version becomes 3.9 and since we test all dependencies with 3.8 elsewhere anyway.

[bsipocz]

Weirdly the oldest dependency tests run into issues. I suppose some of the reasons is that old mpl is not providing wheels for python3.8, and something goes somewhat wrong during the build process during CI (the same version seems to be happy with a newer numpy).

On the other hand, following the logic of APE18, NEP29, SPEC0 and alike we should really update the mpl min version requirement, and drop mpl 3.0.x, too.
Following a 2year drop cycle, according to the figure here, we may be able to drop 3.0 and 3.1, too: https://scientific-python.org/specs/spec-0000/

The direction that other packages are going is to synchronise and follow SPEC0 above, which has the 2-year cycle. We haven't included any policy in APE18 beyond NumPy and CPython, however, I see no reason to have a more flexible policy for optional dependencies than for our mandatory ones.

How does everybody feel about it?

cc @astrofrog @Cadair @larrybradley as visualization maintainers

[bsipocz]

(btw, locally I couldn't reproduce the issue, so the mpl version may be a red herring)

[pllim]

ImportError: numpy.core.multiarray failed to import

Yeah, I don't know what happened there. I saw the same error in CircleCI when I tried to pin both matplotlib and numpy. The only way I know how to fix it is unpin numpy, as you said.

I don't mind bumping the minversion but I don't feel like updating the CircleCI truths so soon again... 😬

Or maybe we just don't test with matplotlib in oldest deps Action CI, since CircleCI already does it.

[pllim]

This resulted in malformed sources.list . Do we even need this command now? Not sure.

[bsipocz]

Is there a favourite post-action you use for debugging? E.g. I don't see why line7 on that file should have any issue, etc.

[pllim]

I didn't set this up, so maybe @astrofrog can advise...

[bsipocz]

you're right, based on 0ff2f4f it can be nuked.

[bsipocz]

@olebole - is wcslib 7.6 available for the exotic architectures? the backport way we used before didn't work for bullseye, but the default wcslib version still seems to be 7.4, and therefore the exotic jobs run into the warning from #11883

[olebole]

@bsipocz I still didn't update wcslib in Debian; it is still 7.4 everywhere (we had a freeze until last week, and I still didn't went through all updates yet). I will do this in the next days (weekend or early next week), if that is OK. My previous experience with wcslib portability is excellent; so I expect to have it everywhere within one step.

[olebole]

@bsipocz The newest wcslib, 7.7 is now uploaded to Debian (already migrated to testing today). astropy 4.3.1 seems happy with it 👓

[pllim]

Thanks for the update! I restarted the affected jobs... 🤞

[pllim]

Did I restart the job too early? I still see this:

Get:153 .../debian bullseye/main arm64 wcslib-dev arm64 7.4+ds-2 [367 kB]

[olebole]

Bullseye comes with 7.4, and will remain so. You need to switch to testing (or bookworm) instead.

[pllim]

@olebole , what we can use is limited by what is provided at https://github.com/uraimo/run-on-arch-action#supported-platforms . I don't see "testing" or "bookworm". Any advise?

[olebole]

Hmm, I could backport the new wcslibs to bullseye. This would require that these platforms include the bullseye-backports repository, and one needs to specify the version in apt.

[pllim]

@olebole , that might work if we can reapply a patch like this here:

0ff2f4f

[olebole]

Yep. that probably would work (with s/buster/bullseye/). However, I need a few more days to get that ready, and it needs the formal acceptance by the ftp-masters.

[pllim]

@olebole , any update on the status of backport? Thanks!

[olebole]

@pllim Thank you for the reminder. I uploaded it, and it now needs approval by our ftp-masters.

[pllim]

@olebole , any timeline on when your patch would be accepted and deployed? Thanks!

I am thinking of just merging this PR by the end of today and let the cron job fail until Debian can provide this backport. This is because this PR has been open long enough and I don't want it to have merge conflicts.

[olebole]

That is hard to predict. However, as their backports queue currently contains entries that are only one day old, I guess it will happen in the next days. Sorry, but the ftp-master require some patience, even from us Debian Developers :-)

[pllim]

Okay, let's just merge then. I'll open a follow-up issue (#12144). Thanks for the update! 😄