tools/find_deprecated.py for finding deprecation decorators (and remember to remove them :) ) #10178
Conversation
|
One or more of the the following people are requested to review this:
|
tools/find_deprecated.py
Outdated
| self._since = None | ||
|
|
||
| @property | ||
| def type(self): |
There was a problem hiding this comment.
It's typically better to avoid type in Python because type() is a builtin and Python is permissive with letting you shadow the name, which can cause issues.
I usually go with kind instead as a synonym.
There was a problem hiding this comment.
Good catch! removing the method in 0be2969, since I mostly used for debugging while writing.
| import requests | ||
|
|
||
|
|
||
| class Deprecation: |
There was a problem hiding this comment.
Consider adding type hints to this file. While it's indeed only a tool, I encourage treating build-support code as first-class.
Even though we're not using MyPy, type hints will still help with making this code more self-documenting. And it will save future us time if/when we add MyPy.
There was a problem hiding this comment.
with the help of @mrossinek , type hints in 56904d5 !
tools/find_deprecated.py
Outdated
| with open(file_name, encoding="utf-8") as fp: | ||
| code = fp.read() |
There was a problem hiding this comment.
FYI Path(file_name).read_text() is more concise.
There was a problem hiding this comment.
good point. Done in 1ecc5af (with you as co-author)
tools/find_deprecated.py
Outdated
|
|
||
| if __name__ == "__main__": | ||
| collection = DeprecationCollection(Path(__file__).joinpath("..", "..", "qiskit").resolve()) | ||
| # collection.collect_deprecations() |
tools/find_deprecated.py
Outdated
| DATA_JSON["releases"][f"{LAST_MINOR}.0"][0]["upload_time"] | ||
| ) | ||
| except requests.exceptions.ConnectionError: | ||
| print("https://pypi.org/pypi/qiskit-terra/json timeout...") |
There was a problem hiding this comment.
Why continue with the script? The code will be more clear if you early exit with sys.exit(1). Also technically, you should print(..., file=sys.stderr).
There was a problem hiding this comment.
because no internet connect is required. It is more like an optional (I sometimes remove all deprecation on airplanes, for example)
There was a problem hiding this comment.
file=sys.stderr added in 315643e
Co-authored-by: Max Rossmannek <[email protected]>
Pull Request Test Coverage Report for Build 5766314603
💛 - Coveralls |
Co-authored-by: Eric Arellano <[email protected]>
…; branch 'main' of github.com:Qiskit/qiskit-terra into find_deprecated/1
| return original_path | ||
|
|
||
|
|
||
| class Deprecation: |
There was a problem hiding this comment.
It looks like this only has one subclass, DeprecationCall? You can simplify this by merging the two. No need to overly generalize.
There was a problem hiding this comment.
DeprecationDecorator is also a subclass. It helped me to code it.
Pull Request Test Coverage Report for Build 6325542459
💛 - Coveralls |
…mber to remove them :) ) (Qiskit#10178) * tools/find_deprecated.py * spelling * ast import * remove meth:type * type hints Co-authored-by: Max Rossmannek <[email protected]> * sys.stderr * remove old comment * Path(file_name).read_text() Co-authored-by: Eric Arellano <[email protected]> * smaller try block * bullet lists * no identation * double the * cast * calling instead of decorations * in months --------- Co-authored-by: Max Rossmannek <[email protected]> Co-authored-by: Eric Arellano <[email protected]>
The great work that @Eric-Arellano did annotating all the deprecations with
deprecate_funcanddeprecate_argallows to quickly find deprecations that needs removal.This PR adds
tools/find_deprecated.py. This utility finds the deprecation decorators and groups them bysinceparameter. It also checks how many days had passed since the last minor release.At this point, the current result of running this utility is this: