-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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.
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
Enrich tests for putpalette function #7186
Conversation
In tests, I've put the tested image : Tests/images/FudanPed00003_mask.png |
By matplotlib you can view the image correctly: import matplotlib.pyplot as plt
mask = Image.open('/content/PennFudanPed/PedMasks/FudanPed00001_mask.png')
# # each mask instance has a different color, from zero to N, where
# # N is the number of instances. In order to make visualization easier,
# # let's adda color palette to the mask.
fig, axes = plt.subplots(1, 3, figsize=(16, 12))
ax = axes.flatten()
ax[0].imshow(mask, cmap="gray")
ax[0].set_axis_off()
ax[0].set_title("Mask", fontsize=12)
ax[1].imshow(mask, cmap="gray")
ax[1].set_axis_off()
ax[1].set_title("Mask", fontsize=12)
ax[2].imshow(mask, cmap="gray")
ax[2].set_axis_off()
ax[2].set_title("Masked", fontsize=12)
plt.show() |
If you are interested in a workaround, you can just mask = Image.open(valid_file)
mask.load()
mask.putpalette(
[
0,
0,
0, # black background
255,
0,
0, # index 1 is red
255,
255,
0, # index 2 is yellow
255,
153,
0, # index 3 is orange
]
) |
So this is a code smell. Could I fix this? |
Internally it is called load! def putpalette(self, data, rawmode="RGB"):
"""
Attaches a palette to this image. The image must be a "P", "PA", "L"
or "LA" image.
The palette sequence must contain at most 256 colors, made up of one
integer value for each channel in the raw mode.
For example, if the raw mode is "RGB", then it can contain at most 768
values, made up of red, green and blue values for the corresponding pixel
index in the 256 colors.
If the raw mode is "RGBA", then it can contain at most 1024 values,
containing red, green, blue and alpha values.
Alternatively, an 8-bit string may be used instead of an integer sequence.
:param data: A palette sequence (either a list or a string).
:param rawmode: The raw mode of the palette. Either "RGB", "RGBA", or a mode
that can be transformed to "RGB" or "RGBA" (e.g. "R", "BGR;15", "RGBA;L").
"""
from . import ImagePalette
if self.mode not in ("L", "LA", "P", "PA"):
msg = "illegal image mode"
raise ValueError(msg)
if isinstance(data, ImagePalette.ImagePalette):
palette = ImagePalette.raw(data.rawmode, data.palette)
else:
if not isinstance(data, bytes):
data = bytes(data)
palette = ImagePalette.raw(rawmode, data)
self.mode = "PA" if "A" in self.mode else "P"
self.palette = palette
self.palette.mode = "RGB"
self.load() # install new palette |
Yes, I've created PR #7187 to fix this and add a test for it. |
Great. So could you merge this PR? |
You'd like this PR merged? Your test runs your scenario, and then asserts that it failed. This is the opposite of what we typically do, where we assert correct behaviour. If one of our tests fails, that generally indicates that something is broken and action needs to be taken to fix it. If your test were to fail, the only action to take is to remove your test. We do have some tests that are known to fail, using xfail - see https://docs.pytest.org/en/7.3.x/how-to/skipping.html#xfail-mark-test-functions-as-expected-to-fail - but those are scenarios where we don't have a viable fix. My PR does have a fix, and indeed, if my PR is merged, then the test in this PR would start to fail. |
To demonstrate that this would start failing if my PR is merged, I checked out your branch and cherry-picked the commit from my PR onto it. You can see the result here - https://github.com/radarhere/Pillow/actions/runs/5090886719/jobs/9150255542 |
Fine. Merge your branch, after that I'll fix my test |
#7187 has been merged. |
putpalette
methods in masked images raises ValueError. It should be fixed