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

Replace imghdr by puremagic #19074

Open
wants to merge 1 commit into
base: dev
Choose a base branch
from

Conversation

bernt-matthias
Copy link
Contributor

@bernt-matthias bernt-matthias commented Oct 29, 2024

imghdr allowed to detect a few image types and has been deprecated in python 3.13

from the release notes:

The filetype, puremagic, or python-magic libraries should be used as replacements.
For example, the puremagic.what() function can be used to replace the imghdr.what()
function for all file formats that were supported by imghdr.

See also code comment

Since we do not really know how often PIL fails nowadays, we could also drop it. Overall I could imagine that puremagic could help us as well for the datatype sniffing (I think over there we reinvent the wheel from time to time).
The number of supported filetypes and being python only seems also in favor of puremagic (compared to filetype and python-magic)

Stumbled over this here: galaxyproject/bioblend#472

How to test the changes?

(Select all options that apply)

  • I've included appropriate automated tests.
  • This is a refactoring of components with existing test coverage.
  • Instructions for manual testing are as follows:
    1. [add testing steps and prerequisites here if you didn't write automated tests covering all your changes]

License

  • I agree to license these and all my past contributions to the core galaxy codebase under the MIT license.

@github-actions github-actions bot added this to the 24.2 milestone Oct 29, 2024
@bernt-matthias bernt-matthias force-pushed the topic/imghdr branch 2 times, most recently from 713f3de to 9b436f3 Compare October 29, 2024 11:52
imghdr allowed to detect a few imagetimes and has been deprecated
in python 3.13: https://docs.python.org/3.12/library/imghdr.html

from the release notes:

> The filetype, puremagic, or python-magic libraries should be used as replacements.
> For example, the puremagic.what() function can be used to replace the imghdr.what()
> function for all file formats that were supported by imghdr.

See also https://github.com/cdgriffith/puremagic/blob/763349ec4d02ba930fb1142c6eb684afdf06c6ab/puremagic/main.py#L421
@bernt-matthias
Copy link
Contributor Author

bernt-matthias commented Oct 29, 2024

Also dropping the import (as in #18449) at all would be fine for me (was also my first idea). Maybe we can split this out in a separate PR?

I did a few experiments with puremagic (for the sniffers for binary formats) ... which were not really promising (i.e. mostly false negatives).

@nsoranzo
Copy link
Member

Not an expert in this, I am happy to backport my commit dropping imghdr if puremagic isn't that useful.
puremagic is still another requirement that we would need to add to the util package requirement, as you can see from the failing package tests, so we may be better off with pillow any way.

@jmchilton
Copy link
Member

We only seem to use image_util from lib/galaxy/datatypes/images.py - it should probably be moved to galaxy-data - where the requirements make a lot more sense also.

@nsoranzo
Copy link
Member

We only seem to use image_util from lib/galaxy/datatypes/images.py - it should probably be moved to galaxy-data - where the requirements make a lot more sense also.

I did try that, but image_util is also used to define the check_image() function in lib/galaxy/util/checkers.py , which in turn is used in lib/galaxy/datatypes/ (so fine) but also in lib/galaxy/tool_shed/util/tool_util.py, lib/galaxy/tool_util/loader_directory.py and lib/tool_shed/util/commit_util.py .

@bernt-matthias
Copy link
Contributor Author

It has been in datatypes until 2016 #3042

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants