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

Title not displayed when using Image.show(title='...') #5739

Closed
FiReTiTi opened this issue Sep 29, 2021 · 14 comments
Closed

Title not displayed when using Image.show(title='...') #5739

FiReTiTi opened this issue Sep 29, 2021 · 14 comments

Comments

@FiReTiTi
Copy link

FiReTiTi commented Sep 29, 2021

What did you do?

I have a NumPy array representing an image that I import using ``, and then I display it using the method show(title='A Title').

[some code....]
from PIL import Image
imgIm = Image.fromarray(MyArrayImage)
imgIm.show(title="Image")

What did you expect to happen?

I expected to see Image written as a title of the window.

What actually happened?

I got a random text.

What are your OS, Python and Pillow versions?

  • OS: MacOSX
  • Python: 3.8.7
  • Pillow: 8.3.2
@nulano
Copy link
Contributor

nulano commented Sep 29, 2021

This function internally just (indirectly) calls PIL.ImageShow.show.

From https://pillow.readthedocs.io/en/stable/reference/ImageShow.html#PIL.ImageShow.show:

title – Optional title. Not all viewers can display the title.

From the provided list of image viewers, only one is listed as supporting title: https://pillow.readthedocs.io/en/stable/reference/ImageShow.html#PIL.ImageShow.UnixViewer.XVViewer

class XVViewer

The X Viewer xv command. This viewer supports the title parameter.

Edit: As discussed below, the display viewer now has now been updated to support the title parameter.

@FiReTiTi
Copy link
Author

Thanks, so it is a very limited feature.
Wouldn't it deserve to be flagged/asserted when used if the viewer is not the proper one?

@radarhere
Copy link
Member

Ok, how about #5743, which would raise a warning for you, "title argument is not supported by this viewer"?

@hugovk
Copy link
Member

hugovk commented Sep 30, 2021

I don't know if a warning is a good idea:

Say a project uses show and then one person uses it on Linux -- no warning -- but then another uses it on Mac and gets a warning.

There's nothing they can do except maybe raise an issue with that project.

What can the project do? Add some code to only pass a title when on Linux? That's not strictly correct because maybe some Linux viewer will add title at some point.

@radarhere
Copy link
Member

Good point, thanks. Closing #5743

@radarhere
Copy link
Member

@FiReTiTi given that discussion, is there anything further you'd like to be done here?

@FiReTiTi
Copy link
Author

I have an answer about why it does not work, but my feeling is that a fix is necessary.
A feature/option that works only for a single viewer only on Linux seems too limited/restricted and is highly confusing.

What can the project do? Add some code to only pass a title when on Linux? That's not strictly correct because maybe some Linux viewer will add title at some point.

It seems the best option (IMHO).

@radarhere
Copy link
Member

I don't think it's possible to implement this feature for all platforms. We could set the name of the temporary file to the title, but trying to filter out invalid filenames would then become its own problem (see https://stackoverflow.com/a/1976050/4093019 and #3450).

@jrazik
Copy link

jrazik commented Oct 22, 2021

I just face that "bug" today and despite the title option is only used for xv, the display command also knows this option.
Thus just change the get_command_ex function line 192 of ImageShow.py to this will be ok (enough ok for me).

    def get_command_ex(self, file, title=None, **options):
        command = executable = "display"
        if title:
            command += f" -title {quote(title)}"
        return command, executable

@radarhere
Copy link
Member

Ok, I've created PR #5788 to add the title option for "display", as you suggest.

@hugovk
Copy link
Member

hugovk commented Oct 23, 2021

Is there any indication this option has been around long enough that it won't fail on some (modernish) systems?

@radarhere
Copy link
Member

https://web.archive.org/web/20120408021811/http://linux.die.net/man/1/display indicates that it has been there since at least 2012.

@hugovk
Copy link
Member

hugovk commented Oct 23, 2021

Thank you all, merged! Let's close this unless there's more concrete steps to take.

@hugovk hugovk closed this as completed Oct 23, 2021
@nulano
Copy link
Contributor

nulano commented Oct 23, 2021

While updating my comment above (#5739 (comment)) I noticed #5788 didn't update the documentation so I have created #5790 to do that.

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

Successfully merging a pull request may close this issue.

5 participants