Added pending_warning_type and a version to the message for deprecated #16463

nabobalis · 2024-05-16T01:18:56Z

TODO:

Changelog?
Fix the tests

github-actions · 2024-05-16T01:19:21Z

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 instructions for rebase and squash.
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?
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.

nabobalis · 2024-05-16T01:19:34Z

astropy/utils/decorators.py

@@ -28,6 +28,19 @@
 _NotFound = object()


+def _get_removal_version(since):
+    # Work out which version this will be removed in


I did not check if this is valid for astropy.

I agree with Brigitta that this is orthogonal to the initial PR goal and should be left out for now.
I also wouldn't recommend attempting to predict the future in deprecation warnings; I've been bitten by it in the past where an important release got blocked by some deprecation that we promised to conclude (remove the functionality), but turned out to be more difficult than we anticipated.

github-actions · 2024-05-16T01:19:41Z

👋 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?

bsipocz

Looks good to me, pending the message itself is left unchanged and a changelog is added.

My preference would be to have this backported into a bugfix, so it's available for downstream usage sooner.

bsipocz · 2024-05-16T03:26:03Z

astropy/utils/decorators.py

@@ -203,7 +224,7 @@ def deprecate(
            else:
                message = (
                    "The {func} {obj_type} is deprecated and may "
-                    "be removed in a future version."
+                    "be removed in a future version (minimum version {removal_version})."


I would not mix this into this PR as this breaks past behaviour when/if anyone matches with the full message. And I would rather see this feature backported, so more versions can use it.

That is fair, I will remove this change.

neutrinoceros · 2024-05-16T07:35:50Z

astropy/utils/decorators.py

@@ -36,6 +49,7 @@ def deprecated(
    pending=False,
    obj_type=None,
    warning_type=AstropyDeprecationWarning,
+    pending_warning_type=AstropyPendingDeprecationWarning,


It's too late for existing arguments, but I'd suggest making any new argument in public functions keyword-only (not crucial, but helps with deprecation cycles, which this PR is all about 😄)

Suggested change

pending_warning_type=AstropyPendingDeprecationWarning,

*,

pending_warning_type=AstropyPendingDeprecationWarning,

neutrinoceros · 2024-05-16T07:39:40Z

astropy/utils/decorators.py

@@ -28,6 +28,19 @@
 _NotFound = object()


+def _get_removal_version(since):
+    # Work out which version this will be removed in


I agree with Brigitta that this is orthogonal to the initial PR goal and should be left out for now.
I also wouldn't recommend attempting to predict the future in deprecation warnings; I've been bitten by it in the past where an important release got blocked by some deprecation that we promised to conclude (remove the functionality), but turned out to be more difficult than we anticipated.

neutrinoceros · 2024-05-16T07:40:28Z

astropy/io/fits/hdu/base.py

@@ -1625,7 +1625,7 @@ def _verify(self, option="warn"):
        return errs


-@deprecated("v6.0")
+@deprecated("6.0")


orthogonal, but doesn't hurt

It was changed due to my adding the version in the message which needs to be able to parse these tags.

Since I will remove that change, I will be undoing this change as well.

pllim · 2024-05-16T14:10:47Z

this backported into a bugfix

But this is technically new API and I don't see the lack of this being blocker of anything downstream, so I prefer not to backport.

bsipocz · 2024-05-16T15:27:44Z

New API for a developer tool, so there isn't really much benefit in holding it back and thus delaying the ability to use it

Another VScode gitignore

049a886

github-actions bot added io.fits utils labels May 16, 2024

nabobalis commented May 16, 2024

View reviewed changes

bsipocz reviewed May 16, 2024

View reviewed changes

neutrinoceros reviewed May 16, 2024

View reviewed changes

pllim added this to the v7.0.0 milestone May 16, 2024

pllim added the API change PRs and issues that change an existing API, possibly requiring a deprecation period label May 16, 2024

nabobalis force-pushed the dep branch from 4e339a3 to b0d01fc Compare May 16, 2024 16:44

Added pending_warning_type

696e712

nabobalis force-pushed the dep branch from b0d01fc to 696e712 Compare May 16, 2024 18:07

nabobalis marked this pull request as ready for review May 16, 2024 18:07

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added pending_warning_type and a version to the message for deprecated #16463

Added pending_warning_type and a version to the message for deprecated #16463

nabobalis commented May 16, 2024 •

edited

github-actions bot commented May 16, 2024

nabobalis May 16, 2024

neutrinoceros May 16, 2024

github-actions bot commented May 16, 2024

bsipocz left a comment

bsipocz May 16, 2024

nabobalis May 16, 2024

neutrinoceros May 16, 2024

nabobalis May 16, 2024

neutrinoceros May 16, 2024

neutrinoceros May 16, 2024

nabobalis May 16, 2024

pllim commented May 16, 2024

bsipocz commented May 16, 2024

	pending_warning_type=AstropyPendingDeprecationWarning,
	*,
	pending_warning_type=AstropyPendingDeprecationWarning,

Added pending_warning_type and a version to the message for deprecated #16463

Are you sure you want to change the base?

Added pending_warning_type and a version to the message for deprecated #16463

Conversation

nabobalis commented May 16, 2024 • edited

github-actions bot commented May 16, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented May 16, 2024

bsipocz left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

pllim commented May 16, 2024

bsipocz commented May 16, 2024

nabobalis commented May 16, 2024 •

edited