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

Add ImageShow support for GraphicsMagick #5349

Merged
merged 5 commits into from Apr 1, 2021

Conversation

@latosha-maltba
Copy link
Contributor

@latosha-maltba latosha-maltba commented Mar 21, 2021

GraphicsMagick[0] is a "stable/conservative" fork of ImageMagick[1]. It provides the same commands and is almost always interchangable with the corresponding imageMagick command. The ImageMagick command <im-command> <arguments> becomes gm <im-command> <arguments>, e.g. convert file.jpg file.png becomes gm convert file.jpg file.png.

This pull request does two things:

  • Update the test suite to run with either ImageMagick or GraphicsMagick (only test_file_palm.py seems to use it)
  • Update ImageShow to support gm display

[0] http://www.graphicsmagick.org/
[1] http://www.graphicsmagick.org/mission.html

Add support to run the tests using GraphicsMagick's "gm convert" instead
of ImageMagick's "convert".
@radarhere radarhere changed the title Add support for GraphicsMagick instead of ImageMagick Add support for GraphicsMagick Mar 22, 2021
@hugovk
Copy link
Member

@hugovk hugovk commented Mar 22, 2021

Thank you for the PR, please could you also include this in the release notes?

https://github.com/python-pillow/Pillow/blob/master/docs/releasenotes/8.2.0.rst

@latosha-maltba
Copy link
Contributor Author

@latosha-maltba latosha-maltba commented Mar 22, 2021

Sure!

Sidenote: Maybe its point worth adding this suggestion (i.e. when providing a pull request also update the releasenotes) to .github/CONTRIBUTING.md?

@latosha-maltba
Copy link
Contributor Author

@latosha-maltba latosha-maltba commented Mar 22, 2021

The Docs fail in Test / ubuntu-latest Python 3.9 (pull_request) and I guess the culprit is the warning WARNING: py:class reference target not found: PIL.ImageShow.GmDisplayViewer.

Sadly I have no clue how to correctly reference the newly added class. The usual :py:class:`PIL.ImageShow.GmDisplayViewer` does not seem to work (might be because of autoclass:: since in the same file :py:class:`PIL.ImageShow.Viewer` does work?). Help would be appreciated.

@nulano
Copy link
Contributor

@nulano nulano commented Mar 22, 2021

Looking at the generated docs, I see that the link to the new viewer references PIL.ImageShow.UnixViewer.GmDisplayViewer: https://pillow--5349.org.readthedocs.build/en/5349/reference/ImageShow.html#PIL.ImageShow.UnixViewer.GmDisplayViewer

Therefore a quick hack would be to use :py:class:`~PIL.ImageShow.UnixViewer.GmDisplayViewer`
I'm not sure whether there is an obvious way to fix this properly without sacrificing the formating.

@latosha-maltba latosha-maltba force-pushed the master branch 2 times, most recently from 523c8c3 to 349b0fb Mar 22, 2021
@latosha-maltba
Copy link
Contributor Author

@latosha-maltba latosha-maltba commented Mar 22, 2021

@nulano Thanks for the hint (in particular the URL to the preliminary generated docs) and solving the mystery. It seems expected and unexpected that Sphinx uses its own class hierarchy instead of the one of Python.

In the end I settled with :py:class:`PIL.ImageShow.GmDisplayViewer <PIL.ImageShow.UnixViewer.GmDisplayViewer>` so we have the full-qualified name (PIL.ImageShow.GmDisplayViewer) pointing to the correct (sub)class.

Interesting side note: Test / ubuntu-latest Python 3.9 (pull_request) failed the docs test due to the warning while docs/readthedocs.org:pillow succeeded...

@radarhere radarhere changed the title Add support for GraphicsMagick Add ImageShow support for GraphicsMagick Mar 22, 2021
Co-authored-by: Andrew Murray <radarhere@users.noreply.github.com>
@hugovk
Copy link
Member

@hugovk hugovk commented Mar 23, 2021

Sidenote: Maybe its point worth adding this suggestion (i.e. when providing a pull request also update the releasenotes) to .github/CONTRIBUTING.md?

Good idea! We already had it in https://github.com/python-pillow/Pillow/blob/master/docs/releasenotes/index.rst but makes sense to include it in the contrib guide too. Please see PR #5357. Thanks!

@hugovk hugovk merged commit 8c852e4 into python-pillow:master Apr 1, 2021
48 of 50 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

4 participants