Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

TST: Uncomment an assert #3053

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open

TST: Uncomment an assert #3053

Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
0ca360e
TST: Uncomment an assert
j-t-1 Jan 14, 2025
bf49726
TST: Uncomment an assert
j-t-1 Jan 14, 2025
542fd80
TST: Uncomment an assert
j-t-1 Jan 14, 2025
d0b9fc7
TST: Uncomment an assert
j-t-1 Jan 14, 2025
c504ef2
TST: Uncomment an assert
j-t-1 Jan 14, 2025
d12c426
TST: Uncomment an assert
j-t-1 Jan 14, 2025
02059e7
TST: Uncomment an assert
j-t-1 Jan 15, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 8 additions & 10 deletions tests/test_filters.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
from .test_images import image_similarity

filter_inputs = (
# "", '', """""",
string.ascii_lowercase,
string.ascii_uppercase,
string.ascii_letters,
Expand Down Expand Up @@ -76,8 +75,7 @@ def test_flatedecode_unsupported_predictor():
def test_flate_decode_decompress_with_array_params(params):
"""FlateDecode decode() method works correctly with array parameters."""
codec = FlateDecode()
s = ""
s = s.encode()
s = b""
encoded = codec.encode(s)
with pytest.raises(DeprecationError):
assert codec.decode(encoded, params) == s
Expand Down Expand Up @@ -169,7 +167,7 @@ def test_ascii85decode_five_zero_bytes():
"""
ASCII85Decode handles the special case of five zero bytes correctly.

From ISO 32000 (2008) §7.4.3:
ISO 32000-1:2008 §7.4.3:

«As a special case, if all five bytes are 0, they shall be represented by
the character with code 122 (z) instead of by five exclamation points
Expand Down Expand Up @@ -355,13 +353,13 @@ def test_png_transparency_reverse():
"""Cf issue #1599"""
pdf_path = RESOURCE_ROOT / "labeled-edges-center-image.pdf"
reader = PdfReader(pdf_path)
_refimg = Image.open(
refimg = Image.open(
BytesIO(get_data_from_url(name="labeled-edges-center-image.png"))
)
data = reader.pages[0].images[0]
_img = Image.open(BytesIO(data.data))
img = Image.open(BytesIO(data.data))
assert ".jp2" in data.name
# assert list(img.getdata()) == list(refimg.getdata())
assert list(img.getdata()) == list(refimg.getdata())


@pytest.mark.enable_socket
Expand Down Expand Up @@ -394,7 +392,7 @@ def test_tiff_predictor():

@pytest.mark.enable_socket
def test_rgba():
"""Decode rgb with transparency"""
"""Decode RGB with transparency"""
with PILContext():
reader = PdfReader(BytesIO(get_data_from_url(name="tika-972174.pdf")))
data = reader.pages[0].images[0]
Expand All @@ -407,7 +405,7 @@ def test_rgba():

@pytest.mark.enable_socket
def test_cmyk():
"""Decode cmyk"""
"""Decode CMYK"""
# JPEG compression
try:
from Crypto.Cipher import AES # noqa: F401
Expand Down Expand Up @@ -475,7 +473,7 @@ def test_index_lookup():
assert data.image.mode == "RGB"
assert image_similarity(data.image, refimg) > 0.999
# indexed CMYK images
# currently with a TODO as we convert to RBG the palette
# currently with a TODO as we convert to RGB the palette
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment still sounds a bit odd ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not sure what the comment means. Do you think it's to do with _handle_flate?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There apparently is/was a TODO comment somewhere as the color palette is/was converted to RGB in this case.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

#2039

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pubpub-zz need your help here. The comment four lines below also please.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What is your issue here? We just convert the palette to RGB - see

pypdf/pypdf/_xobj_image_helpers.py

Lines 245 to 257 in c87f75f

# TODO : cf https://github.com/py-pdf/pypdf/pull/2039
# this is a work around until PIL is able to process CMYK images
elif mode == "CMYK":
_rgb = []
for _c, _m, _y, _k in (
lookup[n : n + 4] for n in range(0, 4 * (len(lookup) // 4), 4)
):
_r = int(255 * (1 - _c / 255) * (1 - _k / 255))
_g = int(255 * (1 - _m / 255) * (1 - _k / 255))
_b = int(255 * (1 - _y / 255) * (1 - _k / 255))
_rgb.append(bytes((_r, _g, _b)))
lookup = b"".join(_rgb)
mode = "RGB"
in the code as well. For Pillow>=10.1.0, this might be changed: python-pillow/Pillow@5957f10

What happens if you enable the assertion in line 480?

Copy link
Contributor Author

@j-t-1 j-t-1 Jan 15, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more. 

FAILED tests/test_filters.py::test_index_lookup - AssertionError: assert 'RGBA' == 'PA'
  
  - PA
  + RGBA

This may be outdated, "PA" is palette? But now we use "P"?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay - I must have missed this. The original comment is still correct - we currently use RGB here instead (or RGBA, if you consider the alpha channel). Apparently, it should be P/PA instead (P is a color mode which uses a (custom) palette to map to another color mode, PA is the variant with an alpha channel).

Upstream documentation of color modes: https://pillow.readthedocs.io/en/stable/handbook/concepts.html#modes

reader = PdfReader(BytesIO(get_data_from_url(name="tika-972174.pdf")))
refimg = Image.open(BytesIO(get_data_from_url(name="usa.png")))
data = reader.pages[0].images["/Im3"]
Expand Down
Loading