Integrating `filetype` as `imghdr` is deprecated and slated for removal in Python3.13

[kengoon]

trafficstars

Refactored and updated the Imghdr code to maintain compatibility with Kivy. This adjustment is necessary as Imghdr is slated for removal in Python 3.13. fix #8294

Maintainer merge checklist

• [ ] Title is descriptive/clear for inclusion in release notes.
• [ ] Applied a Component: xxx label.
• [ ] Applied the api-deprecation or api-break label.
• [ ] Applied the release-highlight label to be highlighted in release notes.
• [ ] Added to the milestone version it was merged into.
• [ ] Unittests are included in PR.
• [ ] Properly documented, including versionadded, versionchanged as needed.

[welcome[bot]]

Thanks for opening your first pull request here! 💖 Please check out our contributing guidelines.

[p0lygun]

First off congrats on the first PR 🥳 🎉 🎉 🎉 secondly @kengoon why have you opted to make our own module instead of using filetype ? im just asking to know the reason, whether its the correct decision or not is upto the maintainers

[p0lygun]

i proposed filetype because it is the "official" recommended alternative

[kengoon]

@p0lygun, Kivy's current implementation utilizes imghdr exclusively for image type checks within the ImageLoader module. Given the singular usage and the fact that it specifically focuses on image types, incorporating an additional dependency seems unnecessary.

Could you share insights into any prospective plans for extending file type checks beyond images in the future? If there are no such plans, maintaining imghdr as an internal library might be a pragmatic choice. I'd appreciate hearing your thoughts on the rationale behind considering an external package and the potential benefits it brings to Kivy.

[p0lygun]

My main reason for adding a dependency is that kivy does not need to worry about testing if our implementation is correct, and the package is recommended and maintained, and it makes it much easier to extend the supported file type later on as we don't need to worry about whether we are checking the correct bytes or not.

as for adding a dependency, I don't this it really matters as we will do version pinning either way and its a pure python package so it will not break any existing pipeline

[kengoon]

Though I understand your point, but right now, I'm in a confused state and have to seek extra advice from the gods 😂 since this is my first time making contributions to the kivy framework (whispers: and also the reason I'm afraid of making contributions here).

@misl6 what's your take on this?

[misl6]

Hi @kengoon !

Yes, I completely agree with @p0lygun regarding this topic.

A 20Kb well-mantained plain-python dependency looks good for me.

Obviously, if the alternative was to include a +20MB not-plain Python dependency instead of having ~100 lines of vendored plain-python legacy code, the decision was different.

[kengoon]

Alright let's switch things up

[misl6]

@kengoon I see lib/imagehdr.py is still there, did you missed to remove it?

[kengoon]

@misl6 let me check again, I'm sure I did remove it, maybe I forgot to commit the file

[kengoon]

@misl6 fixed

[misl6]

Hi @kengoon ,

Tests are failing, can you please take care of it?

[kengoon]

@kengoon on it

[kengoon]

@misl6 I added filetype package to base in setup.cfg. But I noticed that in the CI build, inside of install-kivy function that it only installs dev and full.

So, should I move the filetype package to full or include base in the dependencies to be installed during CI build?

[misl6]

@misl6 I added filetype package to base in setup.cfg. But I noticed that in the CI build, inside of install-kivy function that it only installs dev and full.

So, should I move the filetype package to full or include base in the dependencies to be installed during CI build?

Yes. Otherwise users installing it via pip install kivy[full] will not install filetype as a dependency.

[kengoon]

@misl6 fixed

[misl6]

Hi @kengoon !

PEP8 tests are failing, can you take care of it?

[kengoon]

@misl6 alright 👌

[kengoon]

@misl6 I would like to know if there's a way I can automate this test locally on my machine before I commit?

[misl6]

@misl6 I would like to know if there's a way I can automate this test locally on my machine before I commit?

See: https://github.com/kivy/kivy/blob/15f1db0394f85386c35c3e61e9be2ca1270efdc5/.github/workflows/test_ubuntu_python.yml#L17-L20

[misl6]

Hi @kengoon ,

This is a friendly ping, how it's going with this task?

[kengoon]

@misl6 I was going to finish it up over the weekend but we had a complete blackout. Looking forward to completing it between tomorrow or next

[kengoon]

@misl6 done

[p0lygun]

@kengoon did you run tests and pre-commit on your end ? i think there are some linting issues :thinking:

[kengoon]

@p0lygun I'm not good in all this testing thing, please explain what you just said or better still point me to the errors

[misl6]

Tests looks unhappy, can you take care of it?

________________________ ImageTestCase.test_keep_data _________________________

self = <kivy.tests.test_image.ImageTestCase testMethod=test_keep_data>

    def test_keep_data(self):
        root = self.root
        texture = root.texture
        self.assertEqual(root._image._data[0].data, None)
>       i1 = self.cls(self.image, keep_data=True)

kivy\tests\test_image.py:23: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
kivy\core\image\__init__.py:556: in __init__
    self.filename = arg
kivy\core\image\__init__.py:736: in _set_filename
    self._set_filename(value)
kivy\core\image\__init__.py:736: in _set_filename
    self._set_filename(value)
E   RecursionError: maximum recursion depth exceeded in comparison
!!! Recursion detected (same locals & position)
---------------------------- Captured stdout call -----------------------------
D:\a\kivy\kivy\kivy\tests\test_button.png

[kengoon]

Okay

[kengoon]

@misl6 all image related test cases have passed

[welcome[bot]]

Congrats on merging your first pull request! 🎉🎉🎉