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

Added IPythonViewer #5289

Merged
merged 5 commits into from Mar 7, 2021
Merged

Added IPythonViewer #5289

merged 5 commits into from Mar 7, 2021

Conversation

@radarhere
Copy link
Member

@radarhere radarhere commented Feb 26, 2021

Resolves #5286 by adding a new ImageShow.Viewer - IPythonViewer. It calls the display() method to allow ImageShow to work with IPython... and Google Colab.

Here is my demonstration of it working in Google Colab.
Screen Shot 2021-02-26 at 11 09 38 pm

from PIL import Image, ImageShow
im = Image.new("RGB", (100, 100))
im.show()
from PIL import ImageShow
class IPythonViewer(ImageShow.Viewer):
    def show_image(self, image, **options):
        display(image)
        return 1

try:
    from IPython.display import display
except ImportError:
    pass
else:
    ImageShow.register(IPythonViewer)

from PIL import Image
im = Image.new("RGB", (100, 100))
im.show()

# Cleanup
ImageShow._viewers = [v for v in ImageShow._viewers if v.__class__ != IPythonViewer]
@radarhere radarhere force-pushed the ipythonviewer branch 3 times, most recently from 5f1399c to 34b7edf Feb 26, 2021
nulano
nulano approved these changes Feb 26, 2021
Copy link
Contributor

@nulano nulano left a comment

This should be very useful! Even if it takes a while until the changes reach Colab, it is not difficult to upgrade Pillow manually anyway.



try:
from IPython.display import display
Copy link
Contributor

@nulano nulano Feb 26, 2021

Suggested change
from IPython.display import display
from IPython.display import display as _display

Would it be reasonable to add a prefix to avoid confusion when using code completion tools? ImageShow.show and ImageShow.display are very similar and could be confusing to someone using it for the first time.

Copy link
Member Author

@radarhere radarhere Feb 26, 2021

Okay, sure. I went with ipython_display, to be even clearer.

except ImportError:
pass
else:
register(IPythonViewer)
Copy link
Contributor

@nulano nulano Feb 26, 2021

Any reason to put IPythonViewer before the unix viewers but after the other Windows/macOS ones? I think it would be best to put this at the top (or use register(..., order=0)) to help consistency in IPython (e.g. when combined with tools that offer PDF exports, etc.).

Copy link
Member Author

@radarhere radarhere Feb 27, 2021

Yep, no reason. I've moved it up.

try:
viewer.get_command("test.jpg")
except NotImplementedError:
pass
Copy link
Contributor

@nulano nulano Feb 26, 2021

Suggested change
pass
assert viewer.show(hopper()) == 1

Would remove the need for a separate test.

Copy link
Member Author

@radarhere radarhere Feb 27, 2021

The separate test

  • confirms that the IPythonViewer is present when IPython is available
  • runs IPythonViewer even if another Viewer instance has priority

Also, IPython seems to be a special case, where running its Viewer doesn't disrupt the test process - thinking generically for the sake of the future, just because a Viewer isn't based on a command line instruction doesn't mean that we want it to run in the middle of the test suite?

registered. It displays images on all IPython frontends. This will be helpful
to users of Google Colab, allowing ``im.show()`` to display images.

It is lower in priority than the other default Viewer instances, so it will
Copy link
Contributor

@nulano nulano Mar 3, 2021

It is lower in priority than the other default Viewer instances

Please correct me if I'm wrong, but this is false. The new viewer has the highest priority AFAICT

Copy link
Member Author

@radarhere radarhere Mar 3, 2021

You're right! Thanks for catching that. However, the documentation correctly states my intention, so I've updated the code to match it.

@hugovk hugovk merged commit 6108596 into python-pillow:master Mar 7, 2021
48 of 50 checks passed
@radarhere radarhere deleted the ipythonviewer branch Mar 7, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

4 participants