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

Can't set command used for show() #4643

Closed
avalonv opened this issue May 24, 2020 · 12 comments
Closed

Can't set command used for show() #4643

avalonv opened this issue May 24, 2020 · 12 comments

Comments

@avalonv
Copy link

avalonv commented May 24, 2020

I'm trying to change the command used to display the image with show() from display to pqiv but it seems to just ignore any options I put.

f = Image.open(img)
upscaled = f.resize((1000,700), resample=Image.BOX)
upscaled.show(command='pqiv')
  • OS: Void Linux 5.4
  • Python: 3.8
  • Pillow: 6.2.2
@avalonv
Copy link
Author

avalonv commented May 24, 2020

Should probably make a separate issue for this but I'd like to add that as a user with no experience with ImageMagick I find display a really unintuitive default, wouldn't be easy to use xdg-open?

@nulano
Copy link
Contributor

nulano commented May 25, 2020

Looking at git history, the show(command=...) parameter has been unimplemented at least since the PIL fork:

Pillow/PIL/Image.py

Lines 1480 to 1483 in 9a640e3

def show(self, title=None, command=None):
"Display image (for debug purposes only)"
_show(self, title=title, command=command)

Pillow/PIL/Image.py

Lines 2121 to 2127 in 9a640e3

def _show(image, **options):
# override me, as necessary
apply(_showxv, (image,), options)
def _showxv(image, title=None, **options):
import ImageShow
apply(ImageShow.show, (image, title), options)

ImageShow.py ignores all options

The proper way to override the command is to extend one of the ImageShow.Viewers and use their show function, or ImageShow.register it at the top of the list.

The docs are therefore wrong, and the parameter should be deprecated and removed - see #4646.


I believe the default viewer was chosen based on system support, but both the default display and the fallback xv were added all the way back before the PIL fork, with only Ubuntu's eog added later (#2864). Perhaps there are now more modern variants widely available and the list can be updated.

You don't mention what OS you are using, other than "Linux". Can you please be more specific (including version)?

@avalonv
Copy link
Author

avalonv commented May 25, 2020

I am using Void Linux 5.4.

And yeah I see, I tried digging at the code for the show() method and there seemed to be an option to register your own command but I didn't fully grasp the logic. It is not properly implemented but at the same time, it doesn't actually throw an exception if you use it, it just quietly ignores it, which was confusing when coupled with the outdated documentation. I'm a bit surprised I was the first person to report this.

I agree with the idea, the current defaults are awfully outdated, I think perhaps respecting the user's default application for that mimetype would be better.

@nulano
Copy link
Contributor

nulano commented May 25, 2020

... which was confusing when coupled with the outdated documentation. ...

Fun fact, that documentation has always been wrong! 😃

It was added 7 years ago in #280: 25b6371#diff-3fef5f960d5bd8bdfb9c6fadcf96af22L1479-R1461

@radarhere
Copy link
Member

I've created PR #4695 as another option, to actually use the command argument.

@radarhere
Copy link
Member

Fyi, xdg-open has been around since 2006, before the most recent version of PIL was released in 2009.

I'm not opposed to the idea of adding it as another viewer, but could someone make an argument why it should be the default?

@avalonv
Copy link
Author

avalonv commented Jun 14, 2020

I suggested it because xdg-open respects the default application of the system. You don't have to add complex logic to see if the user has a specific image viewer installed, you just tell it to open the default application for that mimetype.

@FergusonTG
Copy link

Hi

I had a play with this and managed:

"""Use geeqie as a PIL.Image.show viewer"""

import PIL.ImageShow as show


class GeeqieViewer(show.UnixViewer):
    """Inherit from given class."""
    # pylint: disable=too-few-public-methods,no-self-use,unused-argument

    def get_command_ex(self, file, **options):
        """Define a shell command line"""
        command = "geeqie -t "  # -t launches without toolbar
        executable = "/usr/bin/geeqie"
        return command, executable


if show.shutil.which('geeqie'):
    show.register(GeeqieViewer, -1)

Incidentally, there seems to be a bug in ImageShow.register:

    if order > 0:
        _viewers.append(viewer)
    elif order < 0:
        _viewers.insert(0, viewer)

which would silently ignore calling with order=0, the obvious way of putting your new class at the head of the list. I think the elif ...: line should be else:

Cheers
Tim

@radarhere
Copy link
Member

@FergusonTG fair enough. I've created PR #4706 to correct this.

@FergusonTG
Copy link

:-)

@radarhere
Copy link
Member

#4646 has been merged, so the command argument is now deprecated.

@hugovk
Copy link
Member

hugovk commented Jul 7, 2020

Let's close this: command doesn't work, we've now deprecated it (#4646), fixed a problem with order (#4706), and use a subclass of ImageShow.Viewer instead.

Thanks for bringing the issue to light!

@hugovk hugovk closed this as completed Jul 7, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants