Adding support for Pillow Image Object in toga #2154

dr-gavitron · 2023-10-14T19:00:45Z

Added a new parameter called `src` in toga.Image that takes any supported image sources

supported image sources: str, pathlib.Path, bytes, BytesIO, BufferedReader, PIL.Image.Image
type of the src is detected using isinstance. Then it is converted to either path or data. Rest of the code is untouched.
Only an overhead is added for the sake of users.

Added support for Pillow Image Object in toga.Image.

toga.Image can take PIL.Image.Image as its source. Internally PIL.Image.Image is saved to a BytesIO using its save method.
then from the BytesIO, bytes are extracted and passed to data parameter.

Added support from PIL.Image in toga.ImageView

a type checking occurs in init which on detect of PIL.Image.Image, converts it to toga.Image by passing it to the same.

implemented toga.Image.as_format

has a parameter format which when not set(i.e. None) returns the same toga.Image object from which it is being called. But when format is set to PIL.Image.Image, it will convert the image by following mean:

Saving it to a tempfile in disk using save method of self.
Loading the tempfile as Pillow Object by calling PIL_Image.open
Deleting the tempfile
returning the pillow object

implemented toga.ImageView.as_image

has a parameter called format. which if None returns toga.Image else if set to PIL_Image.Image, will return Pillow object. Internally calls toga.Image.as_format

modified Canvas.as_image to support PIL.Image return

has parameter format which if none returns toga.Image. if set to PIL_Image.Image return an object of same but this time uses PIL_Image.open(BytesIO) unlike toga.Image.as_format which uses saving temp file to disk. The BytesIO for PIL_Image.open is constructed from self._impl.get_image_data which returns raw bytes.

toga.Image can take any supported image sources as its first parameter #2156
Allow toga to work with Pillow image files #2142

Fixes #2142, Fixes #2156

PR Checklist:

All new features have been tested
All new features have been documented
I have read the CONTRIBUTING.md file
I will abide by the code of conduct

freakboy3742

A few comments inline, in addition to the ones on the main ticket about API design.

Also - this PR will need tests, plus a changenote.

core/src/toga/images.py

core/src/toga/widgets/imageview.py

dr-gavitron · 2023-10-15T05:25:26Z

I will resolve them as you have said. Sorry for the silly mistakes.. Indeed this is my first GitHub contribution.

dr-gavitron · 2023-10-15T15:15:45Z

How to test this features? And how to add a changenote?

freakboy3742 · 2023-10-15T18:58:13Z

See the contribution guide for details on writing a change note.

As for tests - as the changes are all in the core API, you only need to add tests for the core API. The tests are all Pytest test cases; you need to add tests that validate the new code paths you've added - that is, instantiating Image instances using PIL objects. You'll be looking in the following files:

freakboy3742

A good first pass at tests; but a few notes inline.

changes/2142.feature.rst

core/tests/test_images.py

core/tests/widgets/canvas/test_canvas.py

core/tests/widgets/test_imageview.py

freakboy3742 · 2023-10-16T09:05:09Z

core/tests/widgets/test_imageview.py

+def test_as_image_format_is_none():
+    img = toga.Image("resources/toga.png")
+    imageview = toga.ImageView(image=img)
+    img2 = imageview.as_image()


As with Image, this should be an argument error.

wrote tests for unified src Minor changes to code for supporting pillow Fixes beeware#2156

freakboy3742

Some more suggestions inline.

I also strongly suggest you read the rest of the contribution guide I've linked previously - it contains details like how to run the test suite locally, and how to set up pre-commit so that code formatting issues will be corrected before pushing to CI.

core/src/toga/images.py

freakboy3742 · 2023-10-17T06:13:55Z

core/src/toga/images.py

        if path is not None:
            if isinstance(path, Path):
                self.path = path
            else:
                self.path = Path(path)
            self.data = None
-        else:
-            self.path = None
+        if data is not None:


I'm fairly certain this leaves open a path toga.Image() won't raise an error. The test suite should catch this... but the test suite isn't currently running, because of other problems with the test suite.

freakboy3742 · 2023-10-17T06:14:27Z

core/src/toga/images.py

            self.path = toga.App.app.paths.app / self.path
            if not self.path.is_file():
                raise FileNotFoundError(f"Image file {self.path} does not exist")
            self._impl = self.factory.Image(interface=self, path=self.path)

+
+
+


There should be 2 lines between classes. This is picked up automatically when you install pre-commit.

freakboy3742 · 2023-10-17T06:15:39Z

core/src/toga/images.py

+        if format==None:
+            return self
+        elif isinstance(format, type(self)):
+            return self


It's not clear to me why as_format() shouldn't require a single argument - the format you want the image in.

It's also not clear to me why as_format(toga.Image) is something anyone would ever invoke... on toga.Image.

freakboy3742 · 2023-10-17T06:16:34Z

core/src/toga/widgets/canvas.py

+        if format == None:
+            return Image(data=self._impl.get_image_data())
+        elif isinstance(format, toga.Image):
+            return Image(data=self._impl.get_image_data())


This effectively means that format has a default value of toga.Image. This simplifies the type description, and simplifies the implementation because there's no duplicated code path.

freakboy3742 · 2023-10-17T06:17:48Z

core/src/toga/widgets/imageview.py

@@ -63,7 +70,7 @@ def rehint_imageview(image, style, scale=1):
 class ImageView(Widget):
    def __init__(
        self,
-        image: Image | Path | str | None = None,
+        image: Image | Path | str | Any | None = None,


If you're using Any, there's no need to specify anything else, because any type is a subset of Any.

freakboy3742 · 2023-10-17T06:21:06Z

core/src/toga/widgets/imageview.py

@@ -79,6 +86,8 @@ def __init__(
        # Prime the image attribute
        self._image = None
        self._impl = self.factory.ImageView(interface=self)
+        if PIL_Image!=None and PIL_Image.isImageType(image):
+            image = Image(image) #PIL.Image.Image -> toga.Image


As I said in my previous review - this isn't needed. imageview.image = image calls the setter, and if the argument isn't None or a toga.Image(), a toga.Image is constructed. We can completely defer all type checks to the toga.Image() constructor.

freakboy3742 · 2023-10-17T06:21:45Z

core/src/toga/widgets/imageview.py

+        if format==None:
+            return self.image
+        elif isinstance(format, Image):
+            return self.image


If two branches have the same implementation, perhaps consider whether an or is needed.

core/tests/test_images.py

freakboy3742 · 2023-11-27T03:52:03Z

Closing; superseded by #2231

bin-san added 5 commits October 14, 2023 21:53

Added PIL.Image support in toga.Image

134de64

modified imageview to support PIL.Image

811919a

updated canvas

681118e

Changes to images.py

1bf96da

Used Any insted of PIL.Image for type annotations

1ac2e96

dr-gavitron changed the title ~~Fixed #2142~~ Fixed #2142: Allow toga to work with Pillow image files Oct 14, 2023

removed unnecessary codes

7209f5f

freakboy3742 requested changes Oct 15, 2023

View reviewed changes

Resolved all the issues

a7e99b6

wrote tests and added a change note

f809bb6

freakboy3742 requested changes Oct 16, 2023

View reviewed changes

dr-gavitron added 2 commits October 16, 2023 16:08

Created param src that takes any supported sources

280c812

wrote tests for unified src Minor changes to code for supporting pillow Fixes beeware#2156

Resolved some issues

3e4c00c

dr-gavitron changed the title ~~Fixed #2142: Allow toga to work with Pillow image files~~ Allow toga to work with Pillow image files Oct 16, 2023

dr-gavitron changed the title ~~Allow toga to work with Pillow image files~~ Adding support for Pillow Image Object in toga Oct 16, 2023

freakboy3742 mentioned this pull request Oct 16, 2023

toga.Image can take any supported image sources as its first parameter #2156

Closed

freakboy3742 requested changes Oct 17, 2023

View reviewed changes

Made some changes

f371576

freakboy3742 mentioned this pull request Nov 14, 2023

Allow toga to work with Pillow image files #2142

Closed

HalfWhitt mentioned this pull request Nov 27, 2023

PIL Image support #2231

Merged

4 tasks

freakboy3742 closed this Nov 27, 2023

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding support for Pillow Image Object in toga #2154

Adding support for Pillow Image Object in toga #2154

dr-gavitron commented Oct 14, 2023 •

edited

Loading

freakboy3742 left a comment

dr-gavitron commented Oct 15, 2023

dr-gavitron commented Oct 15, 2023

freakboy3742 commented Oct 15, 2023

freakboy3742 left a comment

freakboy3742 Oct 16, 2023

freakboy3742 left a comment

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 Oct 17, 2023

freakboy3742 commented Nov 27, 2023

Adding support for Pillow Image Object in toga #2154

Adding support for Pillow Image Object in toga #2154

Conversation

dr-gavitron commented Oct 14, 2023 • edited Loading

Added a new parameter called src in toga.Image that takes any supported image sources

Added support for Pillow Image Object in toga.Image.

Added support from PIL.Image in toga.ImageView

implemented toga.Image.as_format

implemented toga.ImageView.as_image

modified Canvas.as_image to support PIL.Image return

PR Checklist:

freakboy3742 left a comment

Choose a reason for hiding this comment

dr-gavitron commented Oct 15, 2023

dr-gavitron commented Oct 15, 2023

freakboy3742 commented Oct 15, 2023

freakboy3742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freakboy3742 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

freakboy3742 commented Nov 27, 2023

dr-gavitron commented Oct 14, 2023 •

edited

Loading

Added a new parameter called `src` in toga.Image that takes any supported image sources