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.

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

Allow toga to work with Pillow image files #2142

Closed
freakboy3742 opened this issue Oct 6, 2023 · 23 comments
Closed

Allow toga to work with Pillow image files #2142

freakboy3742 opened this issue Oct 6, 2023 · 23 comments
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!

Comments

@freakboy3742
Copy link
Member

What is the problem or limitation you are having?

Pillow is the defacto image handling library for Python. While it's possible to convert Pillow Image objects into toga.Image objects, it requires some manual boilerplate handling. It should be possible to use Pillow images directly, with the boilerplate being managed by the toga.Image interface.

Describe the solution you'd like

The following code should work:

from PIL import Image
from toga import Image, ImageView, Canvas

with Image.open("picture.jpg") as pil_image:
    # Create an image
    image = toga.Image(pil_image)

    # Construct an image view to display an image
    imageview = toga.ImageView(pil_image)

# Construct a Canvas
canvas = toga.Canvas()

# Convert the Toga image back into a PIL image
new_pil_image = image.as_format(PIL.Image)

# Obtain the Toga ImageView content as a PIL image
new_pil_image = imageview.as_image(format=PIL.Image)

# Obtain the Toga canvas as a PIL image.
canvas_image = canvas.as_image(format=PIL.Image)

The as_image() APIs should default to toga.Image as the format, so imageview.as_image() would be the equivalent of imageview.image.

PIL should not be added as an install-time requirement of toga-core, or any Toga backend. It must be possible use Toga in an environment that doesn't have PIL. This means any from PIL import Image call must be wrapped in except ImportError handling, and the body logic cannot assume that the PIL image class is available.

Describe alternatives you've considered

Do nothing, and require manual conversion.

Additional context

The boilerplate version of converting a PIL image to a Toga image is as follows:

import io
from PIL import Image

with Image.open("picture.jpg") as pil_image:
    buffer = io.BytesIO()
    pil_image.save(buffer, format="png", compress_level=0)
    image_from_bytes = toga.Image(data=buffer.getvalue())

The first argument to toga.Image is the "path". It currently accepts a string or a Path; or we can extend this to allow PIL.Image as a data type, and if PIL is available in the runtime environment, do a type check to perform the conversion into data form. A similar check would be required on the toga.ImageView constructor, the toga.ImageView.image setter, etc

For bonus points, these APIs wouldn't be PIL specific. There are other libraries (e.g., OpenCV) that have their own image formats; it would be desirable to provide support for those image formats as well. The simple solution would be to add additional type checks for other known image formats. An even better option would be to add a plugin interface that allows any third-party library to register an image format that can be passed in to toga.Image et al, with PIL being the first consumer of that API (with the implementation provided as part of Toga).

@freakboy3742 freakboy3742 added enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start! labels Oct 6, 2023
@dr-gavitron
Copy link
Contributor

Hi I'm new to toga and love the project. I want to work on this. Hope I will get your guidance.

@freakboy3742
Copy link
Member Author

@dr-gavitron Awesome - the ticket description should contains all the design details that are specific to this problem; if you're looking for general advice on the Toga development process, we have a section in the docs to get you started.

If you need any more specific advice or guidance, ask a question and we'll do our best to answer it.

@dr-gavitron
Copy link
Contributor

I want to create an ImageView but it did not show up?

def startup(self):
        main_box = toga.Box(style=Pack(flex=1))
        image = toga.Image(self.paths.app / "resources" / "sample.jpg")
        imageview = toga.ImageView(image)
        main_box.add(imageview)
        self.main_window = toga.MainWindow(title=self.formal_name)
        self.main_window.content = main_box
        self.main_window.show()

why I cant see the image? What I can see is a dot, probably one pixel..

@freakboy3742
Copy link
Member Author

That's almost certainly #1745, which has been resolved in the main branch. If you're thinking of working on this feature, you're definitely going to want to be working on the main branch, not the stable release from PyPI.

@dr-gavitron
Copy link
Contributor

dr-gavitron commented Oct 14, 2023

I made some progress. I have added support for PIL.Image in toga.Image.

Here's How

I have added a new argument called pil_image alongside path and data in toga.Image. The default value of pil_image is None.
Also, toga.Image must take one and only one of the args(path, data, pil_image) to be set. If more than one is set, it will return a ValueError. Also, if 3 of them are None, it will also return a value error. This ensures that the source of the image for toga.Image is one and only one.
Now, in the case where pil_image is set (i.e. other two is None), it will save the image in a temporary file. Then load the raw bytes into memory. After a _impl is created from the bytes.

How to use this feature

from PIL import Image
import toga

#loading the image file using pillow
my_pil_img = Image.open("./myimage.png") #this is a PIL.Image

#passing pillow image object to toga
toga_image = toga.Image(pil_image = my_pil_image) #this will create a toga image from the pillow image object

@freakboy3742
Copy link
Member Author

Good to hear you've made progress; the real demonstration that you've made progress is to submit a PR.

Also - it's worth noting that your sample code is different from the sample code that was provided in the feature specficiation. If you've got a good reason for diverging from the original specification, you can make your case for that change - but changing an API from one that has been proposed without a clear explanation is going to raise eyebrows during the review process.

@dr-gavitron
Copy link
Contributor

Inspecting the code I found: toga.Image has 2 args, path and data, both are set to None. So, whenever we will give first argument the path parameter will be set. Like for creating an image with raw bytes

import toga
with open("sample.png", "rb") as f:
    img_bytes = f.read()

toga_image = toga.Image(data=img_bytes)  #this will work
toga_image = toga.Image(img_bytes) #will not work because first positional parameter is `path`

The point is: for now toga.Image has two named argument first one is path and second one is data. Its obvious that for supporting Pillow Image, I respected the way it is, and added a new named parameter called pill_image.
Now, Do you want me to add a feature like this?

toga_img = toga.Image("./myimage.png") #works for paths/str
toga_img = toga.Image(b'rawbytes.....') #works for raw bytes
toga_img = toga.Image(pil_image) #works for pillow image

If you want to do that, then we should unify all the sources then detect the type of the source from the value. For that case
toga.Image will have only one parameter.

...
#toga.Image class
class Image:
    def __init__(
       src: str|Path|PIL.Image|BytesIO|bytes|None=None
    )->None:
....

@dr-gavitron
Copy link
Contributor

Implemented all of the features with little diversion. Now you can:

from PIL import Image
import toga

with Image.open("picture.jpg") as pil_image:
    # Create an image
    image = toga.Image(pil_image=pil_image) #here is the diversion

    # Construct an image view to display an image
    imageview = toga.ImageView(pil_image)

# Construct a Canvas
canvas = toga.Canvas()

# Convert the Toga image back into a PIL image
new_pil_image = image.as_format(PIL.Image)

# Obtain the Toga ImageView content as a PIL image
new_pil_image = imageview.as_image(format=PIL.Image)

# Obtain the Toga canvas as a PIL image.
canvas_image = canvas.as_image(format=PIL.Image)

Submitting a PR.

@freakboy3742
Copy link
Member Author

The point is: for now toga.Image has two named argument first one is path and second one is data. Its obvious that for supporting Pillow Image, I respected the way it is, and added a new named parameter called pill_image. Now, Do you want me to add a feature like this?

toga_img = toga.Image("./myimage.png") #works for paths/str
toga_img = toga.Image(b'rawbytes.....') #works for raw bytes
toga_img = toga.Image(pil_image) #works for pillow image

Allowing a PIL.Image to be passed as the first argument by using a type check is what the "Additional details" section of the feature specification called for.

Adding the equivalent type check for a bytes object to be interpreted as data wasn't part of the original specification, but I think it's sufficiently unambiguous to be worthwhile.

If you want to do that, then we should unify all the sources then detect the type of the source from the value. For that case toga.Image will have only one parameter.

   src: str|Path|PIL.Image|BytesIO|bytes|None=None

Some notes about this:

We can't arbitrarily rename the argument to src. It's currently called path, and changing that name would be a backwards incompatible change. For the same reason, we also need to preserve the data argument. There is existing code that uses an explicit data keyword argument.

If we're going to change the name (and I agree it's probably worth it), we need to maintain a backwards compatibility path. This means the prototype needs to be something like:

Image(
    src: Path|str|bytes|BytesIO|PIL.Image|None=None, 
    *, 
    data: bytes | None=None, 
    path: Path | str | None=None):

with usage of the path argument being transformed into src, raising a DeprecationWarning. See toga.Table for an example of how this is done.

The path and data attributes of the Image should also be preserved, based on what type of input is provided. path will always be a Path, no matter whether the user provides a Path or a string; data will be populated by whatever raw data the user provides.

@dr-gavitron
Copy link
Contributor

In my recent commits, I didnt changed the code. Meaning pil_image is still an argument, but I get your point. There are other image formats that toga may support in the future... So, we cant be giving every time a new argument to toga.Image to support a new format(the way I add pil_image to support pillow image object)

for that case, we should go according to the above way: adding src
It will standardize to have a unified source: paths, string paths, bytes, bytesio, custom third party objects like pillow or open-cv
Also it will be backward compatible.

Should I start implementing it?

@freakboy3742
Copy link
Member Author

Should I start implementing it?

Yes - I think Image et al should take a single argument, and we should use type checking to convert that data into whatever format is possible; src being the name of that argument, with path and data preserved for backwards compatibility reasons.

@dr-gavitron
Copy link
Contributor

Hi. I ran tests but got error.
Where to add PIL as test dependencies?

============================= test session starts ==============================
platform linux -- Python 3.12.0, pytest-7.3.1, pluggy-1.3.0 -- /home/point/Desktop/toga_improv/toga/.tox/py-core/bin/python
cachedir: /home/point/Desktop/toga_improv/toga/.tox/py-core/.pytest_cache
rootdir: /home/point/Desktop/toga_improv/toga/core
configfile: pyproject.toml
plugins: freezegun-0.4.2, asyncio-0.21.0
asyncio: mode=Mode.AUTO
collected 1384 items / 2 errors

==================================== ERRORS ====================================
____________________ ERROR collecting tests/test_images.py _____________________
ImportError while importing test module '/home/point/Desktop/toga_improv/toga/core/tests/test_images.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../.tox/py-core/lib/python3.12/site-packages/_pytest/python.py:617: in _importtestmodule
mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
../.tox/py-core/lib/python3.12/site-packages/_pytest/pathlib.py:564: in import_path
importlib.import_module(module_name)
/usr/lib/python3.12/importlib/init.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
:1381: in _gcd_import
???
:1354: in _find_and_load
???
:1325: in _find_and_load_unlocked
???
:929: in _load_unlocked
???
../.tox/py-core/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:172: in exec_module
exec(co, module.dict)
tests/test_images.py:3: in
from PIL import Image as PIL_Image
E ModuleNotFoundError: No module named 'PIL'
_______________ ERROR collecting tests/widgets/test_imageview.py _______________
ImportError while importing test module '/home/point/Desktop/toga_improv/toga/core/tests/widgets/test_imageview.py'.
Hint: make sure your test modules/packages have valid Python names.
Traceback:
../.tox/py-core/lib/python3.12/site-packages/_pytest/python.py:617: in _importtestmodule
mod = import_path(self.path, mode=importmode, root=self.config.rootpath)
../.tox/py-core/lib/python3.12/site-packages/_pytest/pathlib.py:564: in import_path
importlib.import_module(module_name)
/usr/lib/python3.12/importlib/init.py:90: in import_module
return _bootstrap._gcd_import(name[level:], package, level)
:1381: in _gcd_import
???
:1354: in _find_and_load
???
:1325: in _find_and_load_unlocked
???
:929: in _load_unlocked
???
../.tox/py-core/lib/python3.12/site-packages/_pytest/assertion/rewrite.py:172: in exec_module
exec(co, module.dict)
tests/widgets/test_imageview.py:16: in
from PIL import Image as PIL_Image
E ModuleNotFoundError: No module named 'PIL'
=============================== warnings summary ===============================
../.tox/py-core/lib/python3.12/site-packages/pytest_freezegun.py:17: 2768 warnings
/home/point/Desktop/toga_improv/toga/.tox/py-core/lib/python3.12/site-packages/pytest_freezegun.py:17: DeprecationWarning: distutils Version classes are deprecated. Use packaging.version instead.
if LooseVersion(pytest.version) < LooseVersion('3.6.0'):

-- Docs: https://docs.pytest.org/en/stable/how-to/capture-warnings.html
=========================== short test summary info ============================
ERROR tests/test_images.py
ERROR tests/widgets/test_imageview.py
!!!!!!!!!!!!!!!!!!! Interrupted: 2 errors during collection !!!!!!!!!!!!!!!!!!!!
======================= 2768 warnings, 2 errors in 4.94s =======================
py-core: exit 2 (6.51 seconds) /home/point/Desktop/toga_improv/toga/core> coverage run -m pytest -vv pid=10985
py-core: FAIL code 2 (37.03=setup[0.07]+cmd[30.45,6.51] seconds)
evaluation failed :( (37.15 seconds)

@freakboy3742
Copy link
Member Author

Hi. I ran tests but got error. Where to add PIL as test dependencies?

The set of requirements that must be installed for testing purposes, but not for "normal" installation, is defined in the dev extras in core's setup.cfg

@dr-gavitron
Copy link
Contributor

dr-gavitron commented Oct 20, 2023

Implementation of as_format. Is this correct approach? Saving file to disk and loading again? Because I cant find a way how to save images in a buffer/bytesio. toga.Image has save method that takes file path to save the image. No way to save into a buffer.

def as_format(self, format: Any):
        if PIL_Image != None and format == PIL_Image.Image:
            # saving into temporary file
            unique_time = str(time.time())
            temp_file = f"_toga_Image_as_format_PIL_Image_{unique_time}_"
            temp_file_path = os.path.abspath(os.path.join(toga.App.app.paths.data, temp_file))
            self.save(temp_file_path)
            # creating PIL.Image.Image from temporary file
            pil_image = PIL_Image.open(temp_file_path)
            # deleting the temporary file
            os.remove(temp_file_path)

            return pil_image
        else:
            raise TypeError(f"Unknown conversion format: {format}")

This should be working but tests failed...

FAILED tests/test_images.py::test_as_format - FileNotFoundError: [Errno 2] No such file or directory: '/home/point/user_data/org.beeware.toga.images/toga_Image_as_format_PIL_Image_1697777498.4035635'

I think save did not work properly..!

@freakboy3742
Copy link
Member Author

Is this the correct approach?

No - there's no need to use a temporary file on disk. If the API is file-like, use an io.Bytes object - that provides a file-like interface, but doesn't require creating a temporary file on disk. This is what the .data attribute already returns.

Also - != None will raise a linting error. Comparisons with None should always be is (not) None.

@freakboy3742
Copy link
Member Author

This is what the .data attribute already returns.

I just realised the .data attribute isn't on main - it's part of the in-progress PR #2103. However, the core idea is the same.

@dr-gavitron
Copy link
Contributor

def as_format(self, format: Any):
        if PIL_Image is not None and format == PIL_Image.Image:
            pil_image = PIL_Image.open(self.data)
            return pil_image
        else:
            raise TypeError(f"Unknown conversion format: {format}")

Doing this... I think I should wait,..

@HalfWhitt
Copy link
Contributor

I'd like to take a turn at this, if that's alright.

@freakboy3742 did you have any particular API in mind for registering image formats and their respective converters? Perhaps a registry class and a method like ImageFormats.register that accepts the image format class and two converter functions, one for each direction? (Or the format class and a Converter object that has those as its two methods?)

@dr-gavitron
Copy link
Contributor

dr-gavitron commented Nov 14, 2023

I'd like to take a turn at this, if that's alright.

Yes. I may not be able to work on this any sooner due to some unavoidable circumstances.

@freakboy3742
Copy link
Member Author

@HalfWhitt I'd imagine we'd use importlib.metadata's built-in plugin interface, rather than inventing one of our own. Toga-core would register an entry point for PIL; other image libraries can register their own implementations.

That said:

  1. Adding support for Pillow Image Object in toga #2154 contains a partial implementation that has gone through some initial review; it needs to be rebased against main, but there's definitely content there you could use as a starting point.
  2. It may be advisable to tackle this in two parts - get it working with PIL as a single new source library, then abstract the PIL handling into more generic image library handling interface.

@HalfWhitt
Copy link
Contributor

@dr-gavitron Hope everything's okay, take care of yourself.

@freakboy3742 Aha, the power of asking! I'll read up on that; implementing just PIL first does indeed sound like a good strategy. I did look through the PR, wasn't sure if I should comment there or here. Will start from there.

@freakboy3742
Copy link
Member Author

If you're looking for an examples of how the plugin interface works: Toga already uses them to register backends (see the platform module in toga core, plus the setup.cfg registrations for each backend). Briefcase also makes extensive use of them.

@HalfWhitt HalfWhitt mentioned this issue Nov 17, 2023
4 tasks
@freakboy3742
Copy link
Member Author

Fixed by #2231.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New features, or improvements to existing features. good first issue Is this your first time contributing? This could be a good place to start!
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants