TST: pytest 7.0.0, scipy 1.8.0 broke a few things by pllim · Pull Request #12823 · astropy/astropy

pllim · 2022-02-04T18:25:39Z

Description

This pull request is to fix some stuff that pytest 7.0.0 release broke.

Need new asdf release:

Fix for pytest 7.0 depreciation warning asdf-format/asdf#1075 -- @WilliamJamieson

Out of scope:

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. If this is a manual backport, use the skip-changelog-checks label unless special changelog handling is necessary.
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.

github-actions · 2022-02-04T18:26:20Z

👋 Thank you for your draft pull request! Do you know that you can use [ci skip] or [skip ci] in your commit messages to skip running continuous integration tests until you are ready?

saimn · 2022-02-04T18:44:30Z

Another issue is that pytest.warns(None) is deprecated, because "because many people used it to mean “this code does not emit warnings”, but it actually had the effect of checking that the code emits at least one warning of any type".

❯ rg "pytest.warns\(None\)" | wc -l
35

We should replace those with the solutions advised here: https://docs.pytest.org/en/latest/how-to/capture-warnings.html#additional-use-cases-of-warnings-in-tests

pllim · 2022-02-04T18:57:01Z

Thanks, @saimn ! I'll get to that later today.

dhomeier · 2022-02-04T20:45:02Z

We should replace those with the solutions advised here: https://docs.pytest.org/en/latest/how-to/capture-warnings.html#additional-use-cases-of-warnings-in-tests

For asdf @WilliamJamieson has fixed it in asdf-format/asdf#1075.

dhomeier · 2022-02-04T20:47:11Z

In addition there is an RTD warning/failure due to moved paths (along with the h5py one):

intersphinx inventory has moved: http://docs.h5py.org/en/stable/objects.inv -> https://docs.h5py.org/en/stable/objects.inv
intersphinx inventory has moved: https://pytest.readthedocs.io/en/stable/objects.inv -> https://docs.pytest.org/en/stable/objects.inv

pllim · 2022-02-04T22:33:05Z

astropy/io/fits/tests/test_header.py

            header = hdul[1].header
-        with pytest.warns(None) as warning_list:
-            header["HIERARCH LONG KEYWORD"] = 42
-        assert len(warning_list) == 0


Unless this is emitting a warning that we are purposely ignoring in setup.cfg, we do not need a check.

This probably comes from before the switch to use pytest's warning system. But given that the aim of this test is to test that no warning is emitted, it probably makes sense to check that explicitly ? Or at least add a comment explaining that if a warning is emitted this would fail the test.

Okay, I added them back.

pllim · 2022-02-04T22:34:01Z

docs/conf.py

 intersphinx_mapping['sphinx_automodapi'] = ('https://sphinx-automodapi.readthedocs.io/en/stable/', None)  # noqa: F405, E501
 intersphinx_mapping['packagetemplate'] = ('https://docs.astropy.org/projects/package-template/en/latest/', None)  # noqa: F405, E501
-intersphinx_mapping['h5py'] = ('http://docs.h5py.org/en/stable/', None)  # noqa: F405
+intersphinx_mapping['h5py'] = ('https://docs.h5py.org/en/stable/', None)  # noqa: F405


For the record, the URL in sphinx-astropy is already correct.

pllim · 2022-02-04T22:39:11Z

Hrrmmm... looks like pytest.warns() fails on zero warnings.

pllim · 2022-02-05T04:55:13Z

setup.cfg

    jplephem
    mpmath
-    asdf>=2.6
+    asdf@git+https://github.com/asdf-format/asdf.git


Tests are only passing with this pin, so that means we either have to:

Pin pytest<7

OR

Wait for asdf to release and then bump minversion of asdf to that release.

This pin broke RTD and linkcheck but that is okay because we will not merge with this pin anyway.

pllim · 2022-02-07T23:02:26Z

setup.cfg

    jplephem
    mpmath
-    asdf>=2.6
+    asdf>=2.9.2


Is this too controversial for backport, @nden , @WilliamJamieson , or @eslavich ?

I don't think so, the worst thing someone would encounter upgrading from 2.6 --> 2.9.2 is a deprecation warning. We've been pretty good about staying true to semantic versioning.

astropy/cosmology/tests/test_funcs.py

and replace it with warnings.catch_warnings() or pytest.warns(); the latter fails on zero warning. Bump pytest minversion to 7 because pytest.warns() is new there. Ignore pytest-asdf warning that is already fixed upstream.

cosmology/tests/test_funcs.py warnings changed.

larrybradley · 2022-02-11T16:28:58Z

@pllim The changes in convolution LGTM. Thanks!

nstarman

Cosmology section LGTM!

adrn

Coordinates change looks fine, if it works!

pllim · 2022-02-14T15:08:42Z

I see a few PR merges are getting held up by CI failures. So let's get this in. If the rest of the subpackage maintainers not happy with the diff, they can open follow-up PRs. Thanks!

lumberbot-app · 2022-02-14T15:09:37Z

Owee, I'm MrMeeseeks, Look at me.

There seem to be a conflict, please backport manually. Here are approximate instructions:

Checkout backport branch and update it.

git checkout v5.0.x
git pull

Cherry pick the first parent branch of the this PR on top of the older branch:

git cherry-pick -m1 aecb0de64ea872952fa724061c4896e0df263529

You will likely have some merge/cherry-pick conflict here, fix them and commit:

git commit -am 'Backport PR #12823: TST: pytest 7.0.0, scipy 1.8.0 broke a few things'

Push to a named branch:

git push YOURFORK v5.0.x:auto-backport-of-pr-12823-on-v5.0.x

Create a PR against branch v5.0.x, I would have named this PR:

"Backport PR #12823 on branch v5.0.x (TST: pytest 7.0.0, scipy 1.8.0 broke a few things)"

And apply the correct labels and milestones.

Congratulations — you did some good work! Hopefully your backport PR will be tested by the continuous integration and merged soon!

Remember to remove the Still Needs Manual Backport label once the PR gets merged.

If these instructions are inaccurate, feel free to suggest an improvement.

mhvk · 2022-02-14T15:13:04Z

@pllim - thanks so much for what turned out to be quite the task! (And all great for units)

TST: pytest 7.0.0, scipy 1.8.0 broke a few things

…5.0.x Backport PR #12823 on branch v5.0.x (TST: pytest 7.0.0, scipy 1.8.0 broke a few things)

Using older versions of `pytest` is not possible since astropy/astropy#12823.

Using older versions of `pytest` is not possible since astropy#12823. Furthermore, `pytest-doctestplus` was made compatible with `pytest` 7.0 in scientific-python/pytest-doctestplus#171, so its minimum required version has to be updated too.

pllim added no-changelog-entry-needed backport-v5.0.x labels Feb 4, 2022

pllim added this to the v5.0.2 milestone Feb 4, 2022

github-actions bot added the Docs label Feb 4, 2022

pllim mentioned this pull request Feb 4, 2022

Backport PR #12549 on branch v5.0.x (TST: Temporarily skipping MRT write doctest) #12819

Merged

pllim added testing 🔥 Critical labels Feb 4, 2022

pllim commented Feb 4, 2022

View reviewed changes

pllim mentioned this pull request Feb 4, 2022

DOC: pytest.warns() fails on zero warning pytest-dev/pytest#9611

Merged

pllim force-pushed the pytest-7-fires branch 3 times, most recently from 4fc1b79 to 7259526 Compare February 4, 2022 23:22

pllim added the Extra CI Run cron CI as part of PR label Feb 5, 2022

This comment was marked as resolved.

Sign in to view

pllim force-pushed the pytest-7-fires branch 3 times, most recently from f57db5b to fc86e62 Compare February 5, 2022 04:02

pllim commented Feb 5, 2022

View reviewed changes

pllim mentioned this pull request Feb 7, 2022

Add --doctest-ufunc option to doctest Numpy ufuncs scientific-python/pytest-doctestplus#174

Merged

pllim commented Feb 7, 2022

View reviewed changes

pllim force-pushed the pytest-7-fires branch from 8991f83 to 28e35cd Compare February 7, 2022 23:06

pllim changed the title ~~TST: pytest 7.0.0 broke a few things~~ TST: pytest 7.0.0, scipy 1.8.0 broke a few things Feb 8, 2022

pllim commented Feb 8, 2022

View reviewed changes

astropy/cosmology/tests/test_funcs.py Outdated Show resolved Hide resolved

pllim added 6 commits February 8, 2022 17:17

DOC: Update pytest intersphinx for tmpdir

a358d57

Update intersphinx URLs

0c3d2ff

Stop using pytest.warns(None)

7b98fa6

and replace it with warnings.catch_warnings() or pytest.warns(); the latter fails on zero warning. Bump pytest minversion to 7 because pytest.warns() is new there. Ignore pytest-asdf warning that is already fixed upstream.

Update asdf minversion pin to 2.9.2

ec98ee0

Test changes caused by scipy 1.8.0

bb5eb9c

cosmology/tests/test_funcs.py warnings changed.

Add explicit warning checks back to FITS tests

0eb8d66

pllim force-pushed the pytest-7-fires branch from a737656 to 0eb8d66 Compare February 8, 2022 22:28

Cadair mentioned this pull request Feb 9, 2022

Test pytest-mpl 0.14rc1 #12835

Closed

10 tasks

pllim mentioned this pull request Feb 10, 2022

astropy 5.0 not working with pyinstaller #12837

Closed

emirkmo mentioned this pull request Feb 11, 2022

relax strict dtype checking in ecsv to a warning #12841

Merged

10 tasks

mhvk mentioned this pull request Feb 11, 2022

Check whether arrays are dask arrays while avoiding to import dask. #12806

Merged

9 tasks

nstarman reviewed Feb 13, 2022

View reviewed changes

adrn approved these changes Feb 14, 2022

View reviewed changes

pllim merged commit aecb0de into astropy:main Feb 14, 2022

pllim deleted the pytest-7-fires branch February 14, 2022 15:08

lumberbot-app bot added the Still Needs Manual Backport label Feb 14, 2022

pllim added a commit to pllim/astropy that referenced this pull request Feb 14, 2022

Merge pull request astropy#12823 from pllim/pytest-7-fires

3f958e7

TST: pytest 7.0.0, scipy 1.8.0 broke a few things

pllim mentioned this pull request Feb 14, 2022

Backport PR #12823 on branch v5.0.x (TST: pytest 7.0.0, scipy 1.8.0 broke a few things) #12849

Merged

10 tasks

pllim removed the Still Needs Manual Backport label Feb 14, 2022

pllim added a commit that referenced this pull request Feb 14, 2022

Merge pull request #12849 from pllim/manual-backport-of-pr-12823-on-v…

86e22fe

…5.0.x Backport PR #12823 on branch v5.0.x (TST: pytest 7.0.0, scipy 1.8.0 broke a few things)

eerovaher added a commit to eerovaher/pytest-astropy that referenced this pull request Mar 3, 2022

Require pytest 7.0 or later

1f20704

Using older versions of `pytest` is not possible since astropy/astropy#12823.

eerovaher mentioned this pull request Mar 3, 2022

Require pytest 7.0 or later astropy/pytest-astropy#49

Closed

eerovaher mentioned this pull request Mar 3, 2022

Require pytest 7.0 or later for installation #12914

Merged

10 tasks

dhomeier mentioned this pull request Aug 28, 2023

MNT: more compatibility with NumPy 2.0 #15235

Merged

1 task

Uh oh!

Conversation

pllim commented Feb 4, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist for package maintainer(s)

Uh oh!

github-actions bot commented Feb 4, 2022

Uh oh!

saimn commented Feb 4, 2022

Uh oh!

pllim commented Feb 4, 2022

Uh oh!

dhomeier commented Feb 4, 2022

Uh oh!

dhomeier commented Feb 4, 2022

Uh oh!

pllim Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

saimn Feb 8, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

pllim Feb 4, 2022

Choose a reason for hiding this comment

Uh oh!

pllim commented Feb 4, 2022

Uh oh!

This comment was marked as resolved.

pllim Feb 5, 2022 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

pllim Feb 7, 2022

Choose a reason for hiding this comment

Uh oh!

eslavich Feb 8, 2022

Choose a reason for hiding this comment

Uh oh!

This comment was marked as resolved.

Uh oh!

Uh oh!

larrybradley commented Feb 11, 2022

Uh oh!

nstarman left a comment

Choose a reason for hiding this comment

Uh oh!

adrn left a comment

Choose a reason for hiding this comment

Uh oh!

pllim commented Feb 14, 2022

Uh oh!

lumberbot-app bot commented Feb 14, 2022

Uh oh!

mhvk commented Feb 14, 2022

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

pllim commented Feb 4, 2022 •

edited

Loading

saimn Feb 8, 2022 •

edited

Loading

pllim Feb 5, 2022 •

edited

Loading