Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

How should we add @deprecated to objects in the standard library? #11002

Closed
AlexWaygood opened this issue Nov 9, 2023 · 13 comments · Fixed by #11005
Closed

How should we add @deprecated to objects in the standard library? #11002

AlexWaygood opened this issue Nov 9, 2023 · 13 comments · Fixed by #11005
Labels
project: policy Organization of the typeshed project

Comments

@AlexWaygood
Copy link
Member

AlexWaygood commented Nov 9, 2023

If an object is deprecated in Python 3.12, to be removed in Python 3.14, should we decorate it like this? (1)

@deprecated("Will be removed in Python 3.14")
class Foo: ...

Like this? (2)

@deprecated("Deprecated in Python 3.12; will be removed in Python 3.14")
class Foo: ...

Or like this? (3)

if sys.version_info >= (3, 12):
    @deprecated("Will be removed in Python 3.14")
    class Foo: ...
else:
    class Foo: ...

For somebody who uses Python 3.8, it might be surprising if a type checker started emitting warnings about an object being deprecated if the runtime only emits a DeprecationWarning on Python 3.12+. For that user, Python 3.14 will be a long way away; they might not even be able to switch to the newer way of doing things yet (perhaps the "replacement idiom" only exists on Python 3.9+). For us in typeshed, however, (3) is obviously the most tedious option for us to maintain, as it will mean more branches.

Thoughts?

@JelleZijlstra
Copy link
Member

I think (3) is the right answer in general. In versions before the one that introduces the runtime deprecation, there may not be a suitable replacement, so if we emit warnings, we're not telling users anything actionable.

@sobolevn
Copy link
Member

sobolevn commented Nov 9, 2023

I vote for 2. Otherwise, it would look like:

if sys.version_info >= (3, 9) and sys.version_info < (3, 11):
   @deprecated("Deprecated in Python 3.9; will be removed in Python 3.11")
   def example() -> None: ...
elif sys.version_info < (3, 9):
   def example() -> None: ...

It looks a bit too verbose. I think that deprecation warnings are generally useful. It is up to users to disable them with something like --disable-error-code=deprecation-warning

@sobolevn
Copy link
Member

sobolevn commented Nov 9, 2023

Moreover, users with --no-unused-ignores will have a hard time to conditionally silence these hypothetical warnings. It is present on 3.9+, but not on 3.8-

@srittau
Copy link
Collaborator

srittau commented Nov 9, 2023

I also tend to favor (3), but this is probably something where we need to gain some experience.

@AlexWaygood
Copy link
Member Author

Another question: how do we define "deprecated at runtime"? ast.Num has been deprecated in the ast docs since Python 3.8, but DeprecationWarnings only started being emitted at runtime in Python 3.12.

I also favour (3) generally, but I think it's probably okay for us to have ast.Num decorated with @deprecated on Python 3.8+, since it's been deprecated in the docs since that version?

@AlexWaygood
Copy link
Member Author

Moreover, users with --no-unused-ignores will have a hard time to conditionally silence these hypothetical warnings. It is present on 3.9+, but not on 3.8-

Following the features Ivan introduced in python/mypy#15164, it's actually fairly easy to tackle that problem: you can just do # type: ignore[deprecated,unused-ignore]

@Avasam
Copy link
Collaborator

Avasam commented Nov 9, 2023

Because of the potential difference between typing and runtime, I'd prefer option 2 over option 1. A bit more context and information doesn't hurt the developper.

As for 2 vs 3, here's some of my immediate thoughts, whether or not these are particularly strong arguments:

This only affects static checkers. And can likely be disabled in static checker. A user stuck in a case where this is not replaceable could either add a suppression comment, or disable the check entirely if it's not applicable to them.

Maintainability aside (I think the stdlib should thrive to do "what's right" even if a bit more cumbersome), it is nice to follow the exact runtime implementation, but we have a lot of precedent for not doing so in the context of static typing.

My gut feeling is to promote the most up to date ways of doing things. Which the python type system often offers mechanisms to do so and promote whenever possible (from future import __annotations__, TYPE_CHECKING checks, .pyi stub files being mostly separate from runtime version + flake8-pyi promoting up-to-date standards in them)

I'm also biased for wanting warnings as soon as possible. Maybe option 3 could be used not to differentiate when it's been deprecated, but for when there's no replacement mechanisms?
But then I just run type-checkers in CI on all supported version, which should get me my early warning.


Moreover, users with --no-unused-ignores will have a hard time to conditionally silence these hypothetical warnings. It is present on 3.9+, but not on 3.8-

This has happened to me a handful of times.

Following the features Ivan introduced in python/mypy#15164, it's actually fairly easy to tackle that problem: you can just do # type: ignore[deprecated,unused-ignore]

Ah! Nice, though idk about other checkers than mypy.


Edit: quoting Jelle below:

If the effect of what we're doing is that users simply turn off deprecation warnings, we're doing it wrong.

You may also want to consider what's friendlier to older projects supporting older python versions to adopt static type checking. More early hurdles doesn't help adoption.

@JelleZijlstra
Copy link
Member

Another question: how do we define "deprecated at runtime"? ast.Num has been deprecated in the ast docs since Python 3.8, but DeprecationWarnings only started being emitted at runtime in Python 3.12.

I also favour (3) generally, but I think it's probably okay for us to have ast.Num decorated with @deprecated on Python 3.8+, since it's been deprecated in the docs since that version?

We can probably have @deprecated on more versions in some cases, yes. The wave changes in #11001 might also be a good example, as those methods already did nothing useful in 3.8.

But even in the ast case, what if a user wants to support 3.7 and lower and therefore does need to handle ast.Num? Is it really useful for them to tell that Num is deprecated several versions later?

It looks a bit too verbose. I think that deprecation warnings are generally useful. It is up to users to disable them with something like --disable-error-code=deprecation-warning

If the effect of what we're doing is that users simply turn off deprecation warnings, we're doing it wrong.

@Avasam
Copy link
Collaborator

Avasam commented Nov 9, 2023

Another question: how do we define "deprecated at runtime"? ast.Num has been deprecated in the ast docs since Python 3.8, but DeprecationWarnings only started being emitted at runtime in Python 3.12.

I also favour (3) generally, but I think it's probably okay for us to have ast.Num decorated with @deprecated on Python 3.8+, since it's been deprecated in the docs since that version?

Most stubs won't be throwing a deprecation warning, because the implementation of deprecation wildly varies in the wild. Sometimes is printed, sometimes it's a python warning, sometimes it's just a comment, just in the doc, etc.

I think it's beneficial to mark all those as deprecated in 3rd party-stubs anyway. And I think it would be beneficial for that mentality to be consistent across stdlib and 3rd party stubs. as long as it's properly documented as deprecated for that version (if going for option 3)

@srittau
Copy link
Collaborator

srittau commented Nov 9, 2023

Jelle already mentioned it, but here is a theoretical example, where I wouldn't want the type checker to warn:

if sys.version_info >= (3, 11):
    shiny()  # introduced in 3.11 as alternative for cruddy()
else:
    cruddy()  # deprecated in 3.11

By using version_info in the stubs as well, and only marking the deprecation of cruddy() in Python 3.11+, we'd prevent type checkers from complaining about code like this that explicitly handles deprecations.

@JelleZijlstra
Copy link
Member

Since most of us seem to favor option 3, I submitted #11005 documenting that convention. As @srittau said above, we can revisit this in the future based on user experience.

@hauntsaninja
Copy link
Collaborator

hauntsaninja commented Nov 10, 2023

I think we should default to 2, unless there doesn't yet exist an appropriate replacement (the shiny case), in which case we can fall back to option 3.

Something like:

In the standard library, @deprecated should only be applied on Python versions where an appropriate replacement is available. Otherwise or if in doubt, use sys.version_info branches to restrict @deprecated to Python versions where it is documented as deprecated or issues a warning

Unlike CPython, we have the ability to time travel, and this is a great thing. Concretely, this would have been helpful for me when upgrading my work codebase from 3.9 to 3.11.

@JelleZijlstra
Copy link
Member

#11009 covers a couple more deprecations from the standard library to help guide the decision.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
project: policy Organization of the typeshed project
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants