cmu-cs-academy / desktop-cmu-graphics

BSD 3-Clause "New" or "Revised" License
15 stars 9 forks source link

Allow PIL image to be used as a CMUImage url #75

Open Mohamed-Waiel-Shikfa opened 3 months ago

Mohamed-Waiel-Shikfa commented 3 months ago

Tries to address:

In case the argument passed to checkURL is a PIL image, raises an appropriate less confusing error and suggests a fix.

schmave commented 2 months ago

Thanks for this PR.

I'm using this test code:

from cmu_graphics import *
import os
from PIL import Image as PILImage

USE_MVC = os.environ.get('USE_MVC') == '1'

if USE_MVC:
    def redrawAll(app):
        drawRect(0, 0, 500, 500)
        drawImage(PILImage.open("/Users/evan/Desktop/Screenshot 2024-07-03 at 9.59.07 AM.png"), 0, 0)

    runApp()
else:
    Image(PILImage.open("/Users/evan/Desktop/Screenshot 2024-07-03 at 9.59.07 AM.png"), 0, 0)
    cmu_graphics.loop()

When I run in MVC mode, I see this:

An error occurred. Here is the stack trace:
  "/Users/evan/cpython-cmu-graphics/test.py", line 10:
        drawImage(PILImage.open("/Users/evan/Desktop/Screenshot 2024-07-03 at 9.59.07 AM.png"), 0, 0)
Exception: drawImage only accepts string URLs or instances of CMUImage.
            Did you forget to wrap a PIL image in a CMUImage?

When I run in non-MVC mode, I see this:

An error occurred. Here is the stack trace:
  "/Users/evan/cpython-cmu-graphics/test.py", line 14:
        Image(PILImage.open("/Users/evan/Desktop/Screenshot 2024-07-03 at 9.59.07 AM.png"), 0, 0)
Exception: drawImage only accepts string URLs or instances of CMUImage.
            Did you forget to wrap a PIL image in a CMUImage?

The error message looks good in MVC mode, but in non-MVC mode, we now reference drawImage even though the user didn't use drawImage.

I think instead of changing the error message, it would be nice to automatically wrap PIL Images with PILWrapper, whether you are drawing the drawImage() or Image(). Or if you just want to improve the error message, then this PR would need to be modified so that it shows a different message in non-MVC mode.

Mohamed-Waiel-Shikfa commented 2 months ago

This took me the entirety of yesterday, but it should now be working as excepted and automatically wraps non CMUImage into CMUImage. There might be some small detail that could incite change but I hope you like this progress, and I am open for feedback.