Deprecate deprecations as exceptions and friends by pllim · Pull Request #12633 · astropy/astropy

pllim · 2021-12-24T17:33:45Z

Description

This pull request is to deprecate and remove some old code from core testing infrastructure. I think these should all go together, as it would be more confusing to do them one at a time. 🔥 🔥 🔥

Deprecated	Use this
`astropy.io.ascii.tests.common.raises`	`pytest.raises`
`astropy.tests.helper.raises`	`pytest.raises`
`astropy.tests.helper.catch_warnings`	`pytest.warns`
`astropy.tests.helper.ignore_warnings`	https://docs.pytest.org/en/stable/warnings.html
`astropy.tests.helper.enable_deprecations_as_exceptions`	https://docs.pytest.org/en/stable/warnings.html
`astropy.tests.helper.treat_deprecations_as_exceptions`	https://docs.pytest.org/en/stable/warnings.html

Removed	Use this
`astropy.tests.disable_internet`	`pytest_remotedata.disable_internet`
`astropy.tests.helper.remote_data`	`pytest.mark.remote_data`
`astropy.tests.plugins.display`	`pytest-astropy-header` package

Open follow-up issue to remove stuff deprecated in this PR and milestone it to... 6.0? See MNT: Remove deprecated deprecations as exceptions and friends #12679

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.

pllim · 2021-12-24T17:37:22Z

I see the following at the top of my local run though they did not fail the CI. I cannot figure out where they are coming from. Will have to see if they show up in CI too or is it just me.

WARNING: AstropyDeprecationWarning: The ignore_warnings class is deprecated and may be removed in a future version.
        Use pytest.mark.filterwarnings instead. [pytest_astropy_header.display]
WARNING: AstropyDeprecationWarning: The catch_warnings class is deprecated and may be removed in a future version.
        Use pytest.warns instead. [astropy.tests.helper]

pllim · 2021-12-24T19:02:40Z

Mystery warnings also appear in CI but does not fail the tests.

mhvk

The PR looks good modulo some small nitpicks.

The warnings arise, I think, because the rather old version of pytest_astropy_header imports ignore_warnings (which subclasses from catch_warnings, causing the second warning). We should release a new version!

mhvk · 2021-12-24T21:17:33Z

 def test_wraps():
    """
-    Tests the compatibility replacement for functools.wraps which supports
+    Make sure functools.wraps supports


Shouldn't we just delete the test? Not much point testing the python library...

I don't remember why we had this. I just thought this would at least guard against regression if Python ever changed its behavior in the future. But if no one thinks it is useful, I can remove. Will wait for a second opinion first though.

Astropy's wraps was removed in #7554 so 👍 with removing the test.

mhvk · 2021-12-24T21:21:15Z

@@ -0,0 +1,9 @@
+The following are deprecated and will be removed in a future release.
+Use ``pytest`` warnings handling instead:


maybe "pytest warning and exception handling"

pllim · 2021-12-27T17:35:26Z

I made the grammatical changes suggested by @mhvk and also incorporated #12634 from @nstarman.

The runs should now pick up new pytest-astropy-header 0.2.0 that I just released. So, hopefully mystery warnings seen at the beginning of test runs should be gone? 🤞

mhvk · 2021-12-27T20:01:06Z

It errors on something unrelated:

E     File "/home/runner/work/astropy/astropy/astropy/utils/misc.py", line 41
E       _NOT_OVERWRITING_MSG_MATCH = ('File .* already exists\. If you mean to '
E                                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
E   SyntaxError: invalid escape sequence '\.'

I think this is just a matter of adding making it a raw string, but no idea why this was not caught before!?

eerovaher · 2021-12-27T20:05:48Z

Although the regex template should be built with raw strings, there is something else going on beyond that, see #12641 (comment)

pllim · 2021-12-27T20:14:15Z

I think something in the new pytest-astropy-header release is incompatible. I'll have to investigate.

Update: Might have to just pin it (#12643) until we can figure out astropy/pytest-astropy-header#41

pllim · 2021-12-28T22:09:17Z

I guess I'll try my luck with CI again after #12639 ... 😿

@nstarman

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>

since astropy wraps was removed in astropy#7554

saimn

Looks good to me !

mhvk

Great! Nice removal of old cruft!

pllim · 2021-12-30T22:54:56Z

Don't forget to stop using these in your downstream packages... 😬

pllim · 2021-12-31T07:41:17Z

Let's do this. Thanks for the approvals!

I have started attempts to patch downstream packages for the Astropy Coordinated Packages, infrastrucuture-related packages, and those relevant to my own projects. As for the rest, hopefully people pay attention to deprecation warnings and test their stuff against astropy dev. 🤞

Manual backport of non-controversial parts from #12633

pllim added Refactoring Extra CI Run cron CI as part of PR labels Dec 24, 2021

pllim added this to the v5.1 milestone Dec 24, 2021

pllim requested review from a team, astrofrog and mhvk December 24, 2021 17:33

github-actions Bot added Docs installation io.ascii testing utils labels Dec 24, 2021

mhvk reviewed Dec 24, 2021

View reviewed changes

pllim mentioned this pull request Dec 27, 2021

Unskip missing link #12634

Closed

9 tasks

This comment has been minimized.

Sign in to view

pllim force-pushed the deprecate-deprecations-as-exceptions branch from 4c2f3be to 6b912be Compare December 28, 2021 21:53

nstarman mentioned this pull request Dec 29, 2021

Backport PR #12636 on branch v5.0.x (Add cosmology units to ECSV reading) #12656

Closed

pllim force-pushed the deprecate-deprecations-as-exceptions branch 2 times, most recently from d7342a3 to a979b27 Compare December 29, 2021 17:30

pllim and others added 4 commits December 30, 2021 17:37

Fix another change log typo

cce540f

DOC: Changelog grammars from mhvk

0de951b

Unskip missing link

0c3502c

Signed-off-by: Nathaniel Starkman (@nstarman) <nstarkman@protonmail.com>

TST: Remove test_wraps

416b6c4

since astropy wraps was removed in astropy#7554

pllim force-pushed the deprecate-deprecations-as-exceptions branch from a979b27 to 416b6c4 Compare December 30, 2021 22:39

pllim mentioned this pull request Dec 30, 2021

MNT: Stop using enable_deprecations_as_exceptions spacetelescope/reftools#53

Merged

saimn approved these changes Dec 30, 2021

View reviewed changes

mhvk approved these changes Dec 30, 2021

View reviewed changes

pllim merged commit 82edac5 into astropy:main Dec 31, 2021

pllim deleted the deprecate-deprecations-as-exceptions branch December 31, 2021 07:39

pllim mentioned this pull request Dec 31, 2021

MNT: Remove deprecated deprecations as exceptions and friends #12679

Closed

WilliamJamieson added a commit to WilliamJamieson/poppy that referenced this pull request Jan 3, 2022

Fixes for imports removed by PR: astropy/astropy#12633

911b732

WilliamJamieson mentioned this pull request Jan 3, 2022

Fixes for failures when using astropy develop. spacetelescope/poppy#482

Merged

pllim mentioned this pull request Jan 12, 2022

DOC: Unskip missing wraps from API in v5.0.x #12723

Merged

9 tasks

weaverba137 mentioned this pull request Jan 14, 2022

Remove astropy test helpers weaverba137/pydl#67

Merged

pllim added a commit to pllim/astropy that referenced this pull request Jan 26, 2022

Manual backport of non-controversial parts from astropy#12633

4c5b29e

pllim mentioned this pull request Jan 26, 2022

Manual backport of non-controversial parts from #12633 #12795

Merged

9 tasks

astrofrog added a commit that referenced this pull request Jan 26, 2022

Merge pull request #12795 from pllim/pyinstaller-ugh

3772bef

Manual backport of non-controversial parts from #12633

pllim mentioned this pull request Apr 18, 2023

Deprecate enable_deprecations_as_exceptions #9718

Closed

		@@ -0,0 +1,9 @@
		The following are deprecated and will be removed in a future release.
		Use ``pytest`` warnings handling instead:

Uh oh!

Uh oh!

Conversation

pllim commented Dec 24, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist for package maintainer(s)

Uh oh!

pllim commented Dec 24, 2021

Uh oh!

pllim commented Dec 24, 2021

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

pllim Dec 27, 2021

Choose a reason for hiding this comment

Uh oh!

saimn Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

pllim Dec 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

mhvk Dec 24, 2021

Choose a reason for hiding this comment

Uh oh!

This comment has been minimized.

pllim commented Dec 27, 2021

Uh oh!

mhvk commented Dec 27, 2021

Uh oh!

eerovaher commented Dec 27, 2021

Uh oh!

pllim commented Dec 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

pllim commented Dec 28, 2021

Uh oh!

saimn left a comment

Choose a reason for hiding this comment

Uh oh!

mhvk left a comment

Choose a reason for hiding this comment

Uh oh!

pllim commented Dec 30, 2021

Uh oh!

pllim commented Dec 31, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

pllim commented Dec 24, 2021 •

edited

Loading

pllim commented Dec 27, 2021 •

edited

Loading