Skip to content

Commit

Permalink
MAINT: Use 'page_number' instead of 'pagenum' (#1365)
Browse files Browse the repository at this point in the history
This PR ensures PyPDF2 uses page_number instead of pagenum as a parameter name.

* It does not touch functions that are deprecated anyway.
* The position of the new parameter is the same as the old one
* The old parameter name is added to ensure that people who use the keyword-parameter don't have a breaking change.

Credits to @mtd91429 who pointed out those inconsistencies in #1187
  • Loading branch information
MartinThoma committed Dec 4, 2022
1 parent 3e250c5 commit deb0667
Show file tree
Hide file tree
Showing 7 changed files with 114 additions and 33 deletions.
53 changes: 45 additions & 8 deletions PyPDF2/_merger.py
Expand Up @@ -25,6 +25,7 @@
# ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE
# POSSIBILITY OF SUCH DAMAGE.

import warnings
from io import BytesIO, FileIO, IOBase
from pathlib import Path
from types import TracebackType
Expand Down Expand Up @@ -631,19 +632,20 @@ def find_bookmark(
def add_outline_item(
self,
title: str,
pagenum: int,
page_number: Optional[int] = None,
parent: Union[None, TreeObject, IndirectObject] = None,
color: Optional[Tuple[float, float, float]] = None,
bold: bool = False,
italic: bool = False,
fit: FitType = "/Fit",
*args: ZoomArgType,
pagenum: Optional[int] = None, # deprecated
) -> IndirectObject:
"""
Add an outline item (commonly referred to as a "Bookmark") to this PDF file.
:param str title: Title to use for this outline item.
:param int pagenum: Page number this outline item will point to.
:param int page_number: Page number this outline item will point to.
:param parent: A reference to a parent outline item to create nested
outline items.
:param tuple color: Color of the outline item's font as a red, green, blue tuple
Expand All @@ -653,17 +655,32 @@ def add_outline_item(
:param str fit: The fit of the destination page. See
:meth:`add_link()<add_link>` for details.
"""
if page_number is not None and pagenum is not None:
raise ValueError(
"The argument pagenum of add_outline_item is deprecated. Use page_number only."
)
if pagenum is not None:
old_term = "pagenum"
new_term = "page_number"
warnings.warn(
message=(
f"{old_term} is deprecated as an argument. Use {new_term} instead"
)
)
page_number = pagenum
if page_number is None:
raise ValueError("page_number may not be None")
writer = self.output
if writer is None:
raise RuntimeError(ERR_CLOSED_WRITER)
return writer.add_outline_item(
title, pagenum, parent, color, bold, italic, fit, *args
title, page_number, parent, color, bold, italic, fit, *args
)

def addBookmark(
self,
title: str,
pagenum: int,
pagenum: int, # deprecated, but the whole method is deprecated
parent: Union[None, TreeObject, IndirectObject] = None,
color: Optional[Tuple[float, float, float]] = None,
bold: bool = False,
Expand All @@ -683,7 +700,7 @@ def addBookmark(
def add_bookmark(
self,
title: str,
pagenum: int,
pagenum: int, # deprecated, but the whole method is deprecated already
parent: Union[None, TreeObject, IndirectObject] = None,
color: Optional[Tuple[float, float, float]] = None,
bold: bool = False,
Expand All @@ -708,16 +725,36 @@ def addNamedDestination(self, title: str, pagenum: int) -> None: # pragma: no c
deprecate_with_replacement("addNamedDestination", "add_named_destination")
return self.add_named_destination(title, pagenum)

def add_named_destination(self, title: str, pagenum: int) -> None:
def add_named_destination(
self,
title: str,
page_number: Optional[int] = None,
pagenum: Optional[int] = None,
) -> None:
"""
Add a destination to the output.
:param str title: Title to use
:param int pagenum: Page number this destination points at.
:param int page_number: Page number this destination points at.
"""
if page_number is not None and pagenum is not None:
raise ValueError(
"The argument pagenum of add_named_destination is deprecated. Use page_number only."
)
if pagenum is not None:
old_term = "pagenum"
new_term = "page_number"
warnings.warn(
message=(
f"{old_term} is deprecated as an argument. Use {new_term} instead"
)
)
page_number = pagenum
if page_number is None:
raise ValueError("page_number may not be None")
dest = Destination(
TextStringObject(title),
NumberObject(pagenum),
NumberObject(page_number),
NameObject(TypFitArguments.FIT_H),
NumberObject(826),
)
Expand Down
4 changes: 2 additions & 2 deletions PyPDF2/_reader.py
Expand Up @@ -469,10 +469,10 @@ def getPage(self, pageNumber: int) -> PageObject: # pragma: no cover
"""
.. deprecated:: 1.28.0
Use :code:`reader.pages[pageNumber]` instead.
Use :code:`reader.pages[page_number]` instead.
"""
deprecate_with_replacement(
"reader.getPage(pageNumber)", "reader.pages[pageNumber]"
"reader.getPage(pageNumber)", "reader.pages[page_number]"
)
return self._get_page(pageNumber)

Expand Down
73 changes: 58 additions & 15 deletions PyPDF2/_writer.py
Expand Up @@ -35,6 +35,7 @@
import struct
import time
import uuid
import warnings
from hashlib import md5
from io import BufferedReader, BufferedWriter, BytesIO, FileIO
from pathlib import Path
Expand Down Expand Up @@ -618,10 +619,10 @@ def append_pages_from_reader(
writer_num_pages = len(self.pages)

# Copy pages from reader to writer
for rpagenum in range(reader_num_pages):
reader_page = reader.pages[rpagenum]
for reader_page_number in range(reader_num_pages):
reader_page = reader.pages[reader_page_number]
self.add_page(reader_page)
writer_page = self.pages[writer_num_pages + rpagenum]
writer_page = self.pages[writer_num_pages + reader_page_number]
# Trigger callback, pass writer page as parameter
if callable(after_page_append):
after_page_append(writer_page)
Expand Down Expand Up @@ -1266,19 +1267,20 @@ def addBookmarkDict(
def add_outline_item(
self,
title: str,
pagenum: int,
page_number: Optional[int] = None,
parent: Union[None, TreeObject, IndirectObject] = None,
color: Optional[Union[Tuple[float, float, float], str]] = None,
bold: bool = False,
italic: bool = False,
fit: FitType = "/Fit",
*args: ZoomArgType,
pagenum: Optional[int] = None, # deprecated
) -> IndirectObject:
"""
Add an outline item (commonly referred to as a "Bookmark") to this PDF file.
:param str title: Title to use for this outline item.
:param int pagenum: Page number this outline item will point to.
:param int page_number: Page number this outline item will point to.
:param parent: A reference to a parent outline item to create nested
outline items.
:param tuple color: Color of the outline item's font as a red, green, blue tuple
Expand All @@ -1288,7 +1290,22 @@ def add_outline_item(
:param str fit: The fit of the destination page. See
:meth:`add_link()<add_link>` for details.
"""
page_ref = NumberObject(pagenum)
if page_number is not None and pagenum is not None:
raise ValueError(
"The argument pagenum of add_outline_item is deprecated. Use page_number only."
)
if pagenum is not None:
old_term = "pagenum"
new_term = "page_number"
warnings.warn(
message=(
f"{old_term} is deprecated as an argument. Use {new_term} instead"
)
)
page_number = pagenum
if page_number is None:
raise ValueError("page_number may not be None")
page_ref = NumberObject(page_number)
zoom_args: ZoomArgsType = [
NullObject() if a is None else NumberObject(a) for a in args
]
Expand Down Expand Up @@ -1316,7 +1333,7 @@ def add_outline_item(
def add_bookmark(
self,
title: str,
pagenum: int,
pagenum: int, # deprecated, but the whole method is deprecated
parent: Union[None, TreeObject, IndirectObject] = None,
color: Optional[Tuple[float, float, float]] = None,
bold: bool = False,
Expand Down Expand Up @@ -1380,8 +1397,28 @@ def addNamedDestinationObject(
)
return self.add_named_destination_object(dest)

def add_named_destination(self, title: str, pagenum: int) -> IndirectObject:
page_ref = self.get_object(self._pages)[PA.KIDS][pagenum] # type: ignore
def add_named_destination(
self,
title: str,
page_number: Optional[int] = None,
pagenum: Optional[int] = None,
) -> IndirectObject:
if page_number is not None and pagenum is not None:
raise ValueError(
"The argument pagenum of add_outline_item is deprecated. Use page_number only."
)
if pagenum is not None:
old_term = "pagenum"
new_term = "page_number"
warnings.warn(
message=(
f"{old_term} is deprecated as an argument. Use {new_term} instead"
)
)
page_number = pagenum
if page_number is None:
raise ValueError("page_number may not be None")
page_ref = self.get_object(self._pages)[PA.KIDS][page_number] # type: ignore
dest = DictionaryObject()
dest.update(
{
Expand Down Expand Up @@ -1571,16 +1608,17 @@ def removeText(

def add_uri(
self,
pagenum: int,
page_number: int,
uri: str,
rect: RectangleObject,
border: Optional[ArrayObject] = None,
pagenum: Optional[int] = None,
) -> None:
"""
Add an URI from a rectangular area to the specified page.
This uses the basic structure of :meth:`add_link`
:param int pagenum: index of the page on which to place the URI action.
:param int page_number: index of the page on which to place the URI action.
:param str uri: URI of resource to link to.
:param Tuple[int, int, int, int] rect: :class:`RectangleObject<PyPDF2.generic.RectangleObject>` or array of four
integers specifying the clickable rectangular area
Expand All @@ -1589,7 +1627,12 @@ def add_uri(
properties. See the PDF spec for details. No border will be
drawn if this argument is omitted.
"""
page_link = self.get_object(self._pages)[PA.KIDS][pagenum] # type: ignore
if pagenum is not None:
warnings.warn(
"The 'pagenum' argument of add_uri is deprecated. Use 'page_number' instead."
)
page_number = pagenum
page_link = self.get_object(self._pages)[PA.KIDS][page_number] # type: ignore
page_ref = cast(Dict[str, Any], self.get_object(page_link))

border_arr: BorderArrayType
Expand Down Expand Up @@ -1638,7 +1681,7 @@ def add_uri(

def addURI(
self,
pagenum: int,
pagenum: int, # deprecated, but method is deprecated already
uri: str,
rect: RectangleObject,
border: Optional[ArrayObject] = None,
Expand All @@ -1653,7 +1696,7 @@ def addURI(

def add_link(
self,
pagenum: int,
pagenum: int, # deprecated, but method is deprecated already
pagedest: int,
rect: RectangleObject,
border: Optional[ArrayObject] = None,
Expand Down Expand Up @@ -1685,7 +1728,7 @@ def add_link(

def addLink(
self,
pagenum: int,
pagenum: int, # deprecated, but method is deprecated already
pagedest: int,
rect: RectangleObject,
border: Optional[ArrayObject] = None,
Expand Down
1 change: 1 addition & 0 deletions docs/user/migration-1-to-2.md
Expand Up @@ -163,6 +163,7 @@ utils.py:
* `PdfWriter.get_page`: `pageNumber``page_number`
* `PyPDF2.filters` (all classes): `decodeParms``decode_parms`
* `PyPDF2.filters` (all classes): `decodeStreamData``decode_stream_data`
* `pagenum``page_number`

## Deprecations

Expand Down
2 changes: 1 addition & 1 deletion tests/test_generic.py
Expand Up @@ -578,7 +578,7 @@ def test_remove_child_in_tree():
writer = PdfWriter()
writer._add_object(tree)
writer.add_page(reader.pages[0])
writer.add_outline_item("foo", pagenum=0)
writer.add_outline_item("foo", page_number=0)
obj = writer._objects[-1]
tree.add_child(obj, writer)
tree.remove_child(obj)
Expand Down
8 changes: 4 additions & 4 deletions tests/test_reader.py
Expand Up @@ -415,17 +415,17 @@ def test_get_form(src, expected, expected_get_fields):


@pytest.mark.parametrize(
("src", "page_nb"),
("src", "page_number"),
[
("form.pdf", 0),
("pdflatex-outline.pdf", 2),
],
)
def test_get_page_number(src, page_nb):
def test_get_page_number(src, page_number):
src = RESOURCE_ROOT / src
reader = PdfReader(src)
page = reader.pages[page_nb]
assert reader.get_page_number(page) == page_nb
page = reader.pages[page_number]
assert reader.get_page_number(page) == page_number


@pytest.mark.parametrize(
Expand Down
6 changes: 3 additions & 3 deletions tests/test_writer.py
Expand Up @@ -790,9 +790,9 @@ def test_colors_in_outline_item():
writer = PdfWriter()
writer.clone_document_from_reader(reader)
purple_rgb = (0.50196, 0, 0.50196)
writer.add_outline_item("First Outline Item", pagenum=2, color="800080")
writer.add_outline_item("Second Outline Item", pagenum=3, color="#800080")
writer.add_outline_item("Third Outline Item", pagenum=4, color=purple_rgb)
writer.add_outline_item("First Outline Item", page_number=2, color="800080")
writer.add_outline_item("Second Outline Item", page_number=3, color="#800080")
writer.add_outline_item("Third Outline Item", page_number=4, color=purple_rgb)

target = "tmp-named-color-outline.pdf"
with open(target, "wb") as f:
Expand Down

0 comments on commit deb0667

Please sign in to comment.