MAINT: Refactor Fit / Zoom parameters

[MartinThoma]

Introduce a new PyPDF2.generic.Fit class which captures the type and parameter for how a page should be fit in the viewer (e.g. when clicking on a PDF-internal link).

The class has one method for each fit type which allows users to discover the different types via their IDE, e.g. Fit.xyz(left=123, top=456, zoom=2).

Breaking Change

This PR introduces a breaking change that needs a major version bump. Affected are the following methods:

• PdfMerger.add_outline_item used fit: FitType and *args: ZoomArgType parameters
• PdfWriter.add_outline_item used fit: FitType and *args: ZoomArgType parameters
• AnnotationBuilder.link used fit: FitType = "/Fit" and fit_args: Tuple[ZoomArgType, ...] = tuple() instead.

Instead of having two arguments, we now have only one. To that argument, an object of the new Fit class must be passed.

Why *args is problematic

Using the *args pattern is problematic for two reasons:

• User-code readability:
  • People cannot use the more expressive keyword-argument syntax in their code for the non-fit parameters
  • People may or may not use the keyword-parameter for the fit parameters
• Library extensions: PyPDF2 cannot easily add new parameter to those functions; ENH: Add Cloning  #1371 is the latest PR that stumbled over this issue
• Two parameter for one thing: In general I don't like if we have two parameters for one topic

[codecov]

Codecov Report

Base: 94.14% // Head: 94.14% // Increases project coverage by +0.00% 🎉

Coverage data is based on head (c8b65a4) compared to base (ce0e190).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #1437   +/-   ##
=======================================
  Coverage   94.14%   94.14%           
=======================================
  Files          30       31    +1     
  Lines        5445     5480   +35     
  Branches     1038     1037    -1     
=======================================
+ Hits         5126     5159   +33     
- Misses        191      193    +2     
  Partials      128      128           

Impacted Files	Coverage Δ
PyPDF2/_merger.py	93.18% <ø> (ø)
PyPDF2/_reader.py	90.28% <100.00%> (-0.02%)	⬇️
PyPDF2/_writer.py	89.55% <100.00%> (ø)
PyPDF2/generic/__init__.py	100.00% <100.00%> (ø)
PyPDF2/generic/_annotations.py	100.00% <100.00%> (ø)
PyPDF2/generic/_data_structures.py	95.32% <100.00%> (-0.31%)	⬇️
PyPDF2/generic/_fit.py	100.00% <100.00%> (ø)

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

[MartinThoma]

@pubpub-zz I would introduce this breaking change before #1371. Then you don't need a breaking change in that PR anymore, right?

[MartinThoma]

@pubpub-zz @MasterOdin What do you think about this proposal?

[MasterOdin]

Is there value in allowing fit to be specified as a string for the types that don't require any additional arguments (e.g. /Fit)? Not sure how prevalent usage was for setting this to know how much of a breaking change this is.

If this breaking change does happen, does that mean all of the various deprecated functions and things will be removed as well?

[MartinThoma]

If this breaking change does happen, does that mean all of the various deprecated functions and things will be removed as well?

Good point. Yes, I would proceed with the deprecations then: https://pypdf2.readthedocs.io/en/latest/dev/deprecations.html#how-pypdf2-deprecates-features
I was thinking about adjusting the procedure:

1. x.y.(z+1): Add a DeprecationWarning (currently we use PendingDeprecationWarning
2. (x+1).0.0: Remove / change the code in the breaking way, but keep/add warning / log messages. Here we can help people who didn't look at the warnings before
3. (x+2).0.0: Also remove the warnings - otherwise the code will be pretty bloated at some point.

I am also thinking about finishing some non-breaking changes in PyPDF2. Work on #1187 , finish the work in #1371 .

Then make a cut, make get rid of all of the deprecation warnings / deprecated methods, and move the project to pypdf (without the 2).

For pypdf I would already have ownership on PyPI. We could also think about moving to pdf: idin/pdf#2
I'm pretty confident that we would get the project. I'm just not sure if it would be the best for the ecosystem. In the end pypdf is pretty easy to google, but PDF is just too generic.