Add option to skip load() and copy() if `.convert(...)` is not required

[bromano-oai]

Background

We frequently want to ensure that a given image is in "RGB" mode. To that end, frequently in code .convert("RGB") is invoked immediately after opening an image with Image.open(...).

However, this is wasteful in two ways:

1. This will eagerly trigger load() which may not be necessary if we end up not using the image's bytes
2. This will trigger .copy() where in many cases, a new image is not strictly needed or desired.

    def convert(
        self,
        mode: str | None = None,
        matrix: tuple[float, ...] | None = None,
        dither: Dither | None = None,
        palette: Palette = Palette.WEB,
        colors: int = 256,
    ) -> Image:
        self.load()

        has_transparency = "transparency" in self.info
        if not mode and self.mode == "P":
            # determine default mode
            if self.palette:
                mode = self.palette.mode
            else:
                mode = "RGB"
            if mode == "RGB" and has_transparency:
                mode = "RGBA"
        if not mode or (mode == self.mode and not matrix):
            return self.copy()

Workaround

This can be mitigated by doing conditional check for conversion, but this is a bit verbose.

img = Image.open(...)
if img.mode == "RGB":
  img = img.convert("RGB")

Ask Is there an opportunity to create an alternative API that can ensure the image is in the right mode and perform load() and copy() only when strictly necessary.

[radarhere]

This may be a longer discussion, but here are my thoughts.

This will eagerly trigger load() which may not be necessary if we end up not using the image's bytes

In addition to convert() changing the mode and pixel data, it might also change the info["transparency"] and the palette. It's not possible to determine the new values for them without looking at the pixel data.

If you just want convert() to do almost nothing, and just mark conversion as something that will happen when pixel data, palette or transparency is used, then I think it's an extra layer of complexity for Pillow's internals. I'm wondering why not just wait before you call convert()?

If you're only interested in a few Pillow operations, then maybe you could subclass Image, set your new mode as an variable on the instance, and only convert() internally when you call a second method?

This will trigger .copy() where in many cases, a new image is not strictly needed or desired.

We could add im.convert("RGB", in_place=True) so that the existing image instance is altered instead of creating a new one, but I'm concerned about this idea slowly proliferating through the codebase with any thought about the overall changes. https://github.com/python-pillow/Pillow/pull/7092 already added it for ImageOps.exif_transpose(). Why not for crop(), point() or resize()? Would it not seem a bit complex to have in_place=True in many methods?

[bromano-oai]

I'm wondering why not just wait before you call convert()?

I think primarily the rationale for not deferring convert() is so downstream consumers of the image don't forget to convert into the correct mode. Secondly, doing it directly at the point where the image is used would trigger a copy for each callsite.

I would say the .load() is less of a concern since the likelihood that the image load() ends up being unnecessary is relatively low.

Why not for crop(), point() or resize()? Would it not seem a bit complex to have in_place=True in many methods?

For a service performing many image transformations, it can be quite beneficial to avoid the copies if not strictly needed. It could make sense to have in_place=True in all these scenarios if there is some default case where performing a transformation is unneeded.

[wiredfool]

Exif-transpose's in_place option doesn't really do "in place" so much as it copies the core image object from the transposed image rather than just returning the transposed image. Since it's allocating the core image object and the python object, I don't really see the point.

ImageCMS's in_place is one of those places where it's actually easy to do in place at the C level, since it's f(int32)->int32, there's no memory reshaping necessary. You're just looping over memory.

Any time we change the bounds of the image, or the width of the pixels, we're going to have to allocate, and it's not going to be in place. So crop and resize are right out.

[radarhere]

In my personal opinion, I don't see the need for this request. I think there's a clear way to restructure the user code instead. If many users came and requested this, then we should absolutely reconsider it, but in the meanwhile, I'm not sold.

If you would like this just for your code though, here is my suggestion for a subclass of Image that should do what you want.

from PIL import Image

class DeferredConvertImage(Image.Image):
  _deferred_mode = None
  
  def __init__(self, im: Image.Image) -> None:
    self._normal_im = im
    return super().__init__()
  
  def load(self):
    if self._normal_im:
      if self._deferred_mode:
        self._normal_im = self._normal_im.convert(self._deferred_mode)

      self.im = self._normal_im.im
      self._mode = self._normal_im.mode
      self._size = self._normal_im.size
      if self._normal_im.mode in ("P", "PA"):
        if self._normal_im.palette:
          self.palette = self._normal_im.palette.copy()
        else:
          from . import ImagePalette
  
          self.palette = ImagePalette.ImagePalette()
      self.info = self._normal_im.info.copy()
    self._normal_im = None
    return super().load()
  
  def deferred_convert(self, mode: str) -> None:
    if mode != self._normal_im.mode:
      self._deferred_mode = mode


im = DeferredConvertImage(Image.new("RGB", (100, 100), "#f00"))
im.deferred_convert("L")
im.save("grayscale.png")

[radarhere]

Was my code helpful at all? Or are you really just holding out for a builtin Pillow API for this functionality?

[radarhere]

Since it's allocating the core image object and the python object, I don't really see the point.

I don't think it's allocating the Python level image. https://github.com/python-pillow/Pillow/blob/07fee96880039ad283c4e8154fe4d8786e00adcd/src/PIL/ImageOps.py#L713

[github-actions[bot]]

Closing this issue as no feedback has been received.