MAINT: Deprecate features with PyPDF2==3.0.0 #1489
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #1489 +/- ##
=======================================
Coverage ? 92.03%
=======================================
Files ? 32
Lines ? 5977
Branches ? 1163
=======================================
Hits ? 5501
Misses ? 312
Partials ? 164
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report at Codecov. |
| @@ -1966,7 +1971,7 @@ def xfa(self) -> Optional[Dict[str, Any]]: | |||
|
|
|||
| class PdfFileReader(PdfReader): # pragma: no cover | |||
| def __init__(self, *args: Any, **kwargs: Any) -> None: | |||
| deprecate_with_replacement("PdfFileReader", "PdfReader") | |||
| deprecation_with_replacement("PdfFileReader", "PdfReader", "3.0.0") | |||
There was a problem hiding this comment.
As opposed to having this continue to work just fine, but having a message say that "This was removed in 3.0.0", maybe better to just have this hard error, forcing the user to change? Otherwise, feels weird to have a deprecation message saying some functionality was removed in 3.0.0 and wasn't, and would read to me as a bug of some sort.
There was a problem hiding this comment.
The deprecation_with_replacement raises an exception instead of using warnings.warn. So it will stop working.
The point that I want to address by leaving it here is that (1) many people don't have warnings enabled and (2) a lot of deprecated code still gets written to new tutorials / answers on StackExchange. I see PdfFileParse all the time.
There was a problem hiding this comment.
Ah, yes, I missed the new deprecation utils function and thought these were just calling deprecate with a different string, as well as that the code following this function call wasn't removed. I feel like should maybe remove all code that follows the deprecation call should be removed to better signal that everything ends subsequent to the deprecation call.
I do think having a second release of more strongly removing them while still having it be callable makes sense given the very long tail of Internet tutorials, and also using the opportunity of pypdf to remove it since usage of the new package should be a good separation signal from outdated guides.
There was a problem hiding this comment.
I had two ideas in mind when I didn't remove the function body:
- The diff is a little bit cleaner. It is still very messy, though.
- If anybody really needs it, they could overwrite the deprecation_X functions and the code would still work
- I'm looking forward to a big commit where we remove hundreds of lines 😄
Those arguments are pretty weak, though.
I do think having a second release of more strongly removing them while still having it be callable makes sense given the very long tail of Internet tutorials, and also using the opportunity of pypdf to remove it since usage of the new package should be a good separation signal from outdated guides.
I'm not sure if I can follow. What do you think how we should proceed (with this PR and the versions of PyPDF2/pypdf)?
BREAKING CHANGES: - Deprecate features with PyPDF2==3.0.0 (#1489) - Refactor Fit / Zoom parameters (#1437) New Features (ENH): - Add Cloning (#1371) - Allow int for indirect_reference in PdfWriter.get_object (#1490) Documentation (DOC): - How to read PDFs from S3 (#1509) - Make MyST parse all links as simple hyperlinks (#1506) - Changed 'latest' for 'stable' generated docs (#1495) - Adjust deprecation procedure (#1487) Maintenance (MAINT): - Use typing.IO for file streams (#1498) [Full Changelog](2.12.1...3.0.0)
Deprecate features, but keep helpful exceptions. That means that the names still need to be there. They will stay until 4.0.0. Let's hope the community can transition in that time.