MAINT: Add wrapper function for PendingDeprecationWarnings #928
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Signed-off-by: Matthew Peveler <[email protected]>
MasterOdin
commented
May 30, 2022
| @@ -241,17 +241,13 @@ def paeth_predictor(left: int, up: int, up_left: int) -> int: | |||
| return up_left | |||
|
|
|||
|
|
|||
| def deprecate(msg: str, stacklevel: int = 3) -> None: | |||
| warnings.warn(msg, PendingDeprecationWarning, stacklevel=stacklevel) | |||
There was a problem hiding this comment.
One note is that I believe that this should be changed to a DeprecationWarning as they've been deprecated and will be removed in next major version, where PendingDeprecationWarning should be used for functions that will be deprecated in the future, but are not yet.
There was a problem hiding this comment.
I'm fine with changing it.
As we are now on it, maybe we should specify the process. I would suggest the following if we want to remove a class / function / method. If the version before the change is x.y.z, then:
x.y.(z+1): Add aPendigDeprecationWarning. If there is a replacement, the replacement is also introduced and the warning informs about this change. The docs let users know about the deprecation + the new function. The CHANGELOG informs about it.(x+1).0.0: ThePendingDeprecationWarningis changed to aDeprecationWarning. The CHANGELOG informs about it.(x+2).0.0: The code is actually removed. The CHANGELOG informs about it.
There was a problem hiding this comment.
I've added those thoughts to a PR: #930
Codecov Report
@@ Coverage Diff @@
## main #928 +/- ##
==========================================
+ Coverage 78.25% 78.32% +0.07%
==========================================
Files 16 16
Lines 4346 4342 -4
Branches 821 821
==========================================
Hits 3401 3401
+ Misses 758 754 -4
Partials 187 187
Continue to review full report at Codecov.
|
MartinThoma
reviewed
May 30, 2022
| PendingDeprecationWarning, | ||
| stacklevel=2, | ||
| ) | ||
| deprecate(DEPR_MSG.format(old_name, new_name), 4) |
There was a problem hiding this comment.
Nitpic: I try to use keyword arguments when I use literal parameters, e.g.:
Suggested change
| deprecate(DEPR_MSG.format(old_name, new_name), 4) | |
| deprecate(DEPR_MSG.format(old_name, new_name), stacklevel=4) |
I think this is easier to read.
MartinThoma
approved these changes
May 30, 2022
|
Feel free to merge whenever you want :-) |
MartinThoma
added a commit
that referenced
this pull request
Jun 1, 2022
The 2.0.0 release of PyPDF2 includes three core changes: 1. Dropping support for Python 3.5 and older. 2. Introducing type annotations. 3. Interface changes, mostly to have PEP8-compliant names We introduced a [deprecation process](#930) that hopefully helps users to avoid unexpected breaking changes. Breaking Changes(DEP): - PyPDF2 2.0 requires Python 3.6+. Python 2.7 and 3.5 support were dropped. - PdfFileReader: The "warndest" parameter was removed - PdfFileReader and PdfFileMerger no longer have the `overwriteWarnings` parameter. The new behavior is `overwriteWarnings=False`. - merger: OutlinesObject was removed without replacement. - merger.py ➔ _merger.py: You must import PdfFileMerger from PyPDF2 directly. - utils: * `ConvertFunctionsToVirtualList` was removed * `formatWarning` was removed * `isInt(obj)`: Use `instance(obj, int)` instead * `u_(s)`: Use `s` directly * `chr_(c)`: Use `chr(c)` instead * `barray(b)`: Use `bytearray(b)` instead * `isBytes(b)`: Use `instance(b, type(bytes()))` instead * `xrange_fn`: Use `range` instead * `string_type`: Use `str` instead * `isString(s)`: Use `instance(s, str)` instead * `_basestring`: Use `str` instead * All Exceptions are now in `PyPDF2.errors`: - PageSizeNotDefinedError - PdfReadError - PdfReadWarning - PyPdfError - `PyPDF2.pdf` (the `pdf` module) no longer exists. The contents were moved with the library. You should most likely import directly from `PyPDF2` instead. The `RectangleObject` is in `PyPDF2.generic`. - The `Resources`, `Scripts`, and `Tests` will no longer be part of the distribution files on PyPI. This should have little to no impact on most people. The `Tests` are renamed to `tests`, the `Resources` are renamed to `resources`. Both are still in the git repository. The `Scripts` are now in https://github.com/py-pdf/cpdf. `Sample_Code` was moved to the `docs`. For a full list of deprecated functions, please see the changelog of version 1.28.0. New Features (ENH): - Improve space setting for text extraction (#922) - Allow setting the decryption password in PdfReader.__init__ (#920) - Add Page.add_transformation (#883) Bug Fixes (BUG): - Fix error adding transformation to page without /Contents (#908) Robustness (ROB): - Cope with invalid length in streams (#861) Documentation (DOC): - Fix style of 1.25 and 1.27 patch notes (#927) - Transformation (#907) Developer Experience (DEV): - Create flake8 config file (#916) - Use relative imports (#875) Maintenance (MAINT): - Use Python 3.6 language features (#849) - Add wrapper function for PendingDeprecationWarnings (#928) - Use new PEP8 compliant names (#884) - Explicitly represent transformation matrix (#878) - Inline PAGE_RANGE_HELP string (#874) - Remove unnecessary generics imports (#873) - Remove star imports (#865) - merger.py ➔ _merger.py (#864) - Type annotations for all functions/methods (#854) - Add initial type support with mypy (#853) Testing (TST): - Regression test for xmp_metadata converter (#923) - Checkout submodule sample-files for benchmark - Add text extracting performance benchmark - Use new PyPDF2 API in benchmark (#902) - Make test suite fail for uncaught warnings (#892) - Remove -OO testrun from CI (#901) - Improve tests for convert_to_int (#899) Full Changelog: 1.28.4...2.0.0
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
PR updates all deprecation warnings in the codebase to go through two wrapper functions, to make it easy to keep the message unified, ensure proper
stacklevelused, and that the same type of warning is raised, overall ensuring we're not doing a lot of repeating ourselves. All three of those were different in various places across the codebase, leading to a bit of a confusing experience. The wrapper functions were originally accidentally added in #913, though were unused till now.