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

[py] add deprecation library requirement #13402

Merged
merged 2 commits into from
Jan 20, 2024
Merged

[py] add deprecation library requirement #13402

merged 2 commits into from
Jan 20, 2024

Conversation

titusfortner
Copy link
Member

Description

Add a dependency on deprecation library

Motivation and Context

Current means of doing deprecations in Python is challenging.
Using an annotation would be much easier

Notes

  • This may not be the right fix
  • There were two libraries, this one seems to have fewer features, but its also the one being maintained
  • I think I saw somewhere someone rolling our own annotation, but I'm not sure where, but I seem to remember it feeling clunky?

What do y'all think?

@codecov-commenter
Copy link

codecov-commenter commented Jan 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Comparison is base (b7dc668) 58.23% compared to head (c60948d) 58.23%.
Report is 5 commits behind head on trunk.

❗ Current head c60948d differs from pull request most recent head f9b4c91. Consider uploading reports for the commit f9b4c91 to get more accurate results

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files
@@           Coverage Diff           @@
##            trunk   #13402   +/-   ##
=======================================
  Coverage   58.23%   58.23%           
=======================================
  Files          86       86           
  Lines        5265     5265           
  Branches      222      222           
=======================================
  Hits         3066     3066           
  Misses       1977     1977           
  Partials      222      222           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@diemol diemol left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This should be added. Deprecating something in Python should not be as complicated as it is currently.

@titusfortner
Copy link
Member Author

@isaulv @AutomatedTester @symonk
Do either of you remember seeing a custom implementation? I thought @symonk was doing something, but I can't find it; maybe @sandeepsuryaprasad did something at one point?
Anyway, would like to know if there's an easy win here, and what it is.

@sandeepsuryaprasad
Copy link
Contributor

@titusfortner @symonk @isaulv @AutomatedTester This decorator is implemented in Python 3.13. PEP 702 (https://peps.python.org/pep-0702/). So going forward once python 3.13 is released, we can avoid the dependency on deprecation library since the decorator is already implemented in warnings module https://peps.python.org/pep-0702/#example

@titusfortner
Copy link
Member Author

@sandeepsuryaprasad We have to continue supporting users on 3.12 and below until ~2028.
Is there a way to back port that functionality to earlier versions of Python?

@sandeepsuryaprasad
Copy link
Contributor

@titusfortner yes it is. the new decorator will be added to typing_extensions https://peps.python.org/pep-0702/#backwards-compatibility

@titusfortner
Copy link
Member Author

titusfortner commented Jan 12, 2024

@sandeepsuryaprasad — but this still won't be available until October?

@sandeepsuryaprasad
Copy link
Contributor

@titusfortner Yes.. you are right.. this feature won't be available until October-24.

@titusfortner
Copy link
Member Author

There were two packages I saw:

It looks like the official deprecation implementation in 4.13 is going to be very basic as well, no bells, no whistles. In fact, the syntax looks exactly the same as the Deprecated package syntax.

@AutomatedTester what do you think about using this package, adding the annotations as necessary and then just removing this library and replacing it with the standard one in October?

@AutomatedTester
Copy link
Member

I think this should be fine to add. I am always trying to limit third party but if we feel it will add value (I don't think there is much in terms of deprecation at the moment)

@titusfortner
Copy link
Member Author

Well, there are a few Firefox things that @diemol didn't deprecate because it was complicated. 😂

And I was going to use it to deprecate some Selenium Manager things, but we've decided we can just remove instead of deprecate Selenium Manager things while it is still in beta.

@titusfortner
Copy link
Member Author

Hmm, it looks like we might be able to use this already? from typing_extensions import deprecated??

@titusfortner
Copy link
Member Author

And I verified this works. Nice.

@titusfortner titusfortner merged commit f83765d into trunk Jan 20, 2024
14 checks passed
@titusfortner titusfortner deleted the py_dep branch January 20, 2024 22:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants