picard icon indicating copy to clipboard operation
picard copied to clipboard

PICARD-2354: DeprecationWarning: the imp module is deprecated in favour of importlib

Open skelly37 opened this issue 3 years ago • 2 comments

Summary

The imp module will be removed in Python 3.12 so we have to switch to importlib

  • This is a…
    • [ ] Bug fix
    • [ ] Feature addition
    • [x] Refactoring
    • [ ] Minor / simple change (like a typo)
    • [ ] Other
  • Describe this change in 1-2 sentences:

imp is used only in pluginmanager.py in 2 places, so it doesn't change much of the codebase.

Problem

  • JIRA ticket (optional): PICARD-2354

Solution

I've found some way to replace it in importlib examples but I'm not sure if they're the best way to achieve this.

Action

skelly37 avatar Jul 22 '22 08:07 skelly37

Ref: https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly

skelly37 avatar Jul 22 '22 08:07 skelly37

I'm clueless, I've managed to create a module and to pass it further as the same thing, yet the tests still fail

importlib:

>>> d = importlib.machinery.PathFinder().find_spec("picard", ["/data/it/picard"])
>>> d
ModuleSpec(name='picard', loader=<_frozen_importlib_external.SourceFileLoader object at 0x7fd4e9d9a9e0>, origin='/data/it/picard/picard/__init__.py', submodule_search_locations=['/data/it/picard/picard'])
>>> a = importlib.util.module_from_spec(d)
>>> a
<module 'picard' from '/data/it/picard/picard/__init__.py'>

imp:

>>> i = imp.find_module("picard", ["/data/it/picard"])
>>> i
(None, '/data/it/picard/picard', ('', '', 5))
>>> p = imp.load_module("picard", *i)
>>> p
<module 'picard' from '/data/it/picard/picard/__init__.py'>

skelly37 avatar Jul 22 '22 09:07 skelly37

I made some progress here with https://github.com/metabrainz/picard/pull/2134/commits/fa36ca4b91520191e1032b57e941bfb438b63082 . What was missing was actually executing the module. The newer interfaces of importlib separate loading the module and executing it. Without this all the metadata was not available and the tests failed.

This separation, while not used by Picard currently, actually might help a lot in a future rework of the plugin system, as one of the major downsides of the plugin system right now is that all plugins get executed, even if disabled.

Anyway, the change is not sufficient to make the tests pass. What is still missing is that import picard.plugins.theplugin does not work yet. I'm not sure how to solve this.

Maybe we really need to go another route and we implement plugin loading by doing a custom importer implementation. That probably means implementing a meta path finder and / or a path entry finder as explained in https://docs.python.org/3/library/importlib.html#setting-up-an-importer . I don't yet fully grasp the details here, but currently this looks to me like it would allow us to tackle the issues we still have. Maybe relying more on the capabilities of the importlib API might even simplify the code for loading we currently have.

phw avatar Dec 11 '22 13:12 phw

We will look into this after the PyQt6 changes have been merged. Maybe we need to rethink plugin handling and reimplement parts of it.

phw avatar Jun 12 '23 06:06 phw

We will look into this after the PyQt6 changes have been merged. Maybe we need to rethink plugin handling and reimplement parts of it.

It seems we'll have more to do, since zipimport also has deprecated methods we are using:

zipimporter.find_module() is deprecated and slated for removal in Python 3.12; use find_spec() instead

https://github.com/metabrainz/picard/blob/54337a2dff070b0c4e6ee282a814a073013e34da/picard/pluginmanager.py#L240 https://github.com/python/cpython/blob/171aa086f27e581bfcf2288d5afd0167480ef3cb/Lib/zipimport.py#L146-L160

zas avatar Jun 12 '23 12:06 zas

@zas yes, indeed. We already have https://tickets.metabrainz.org/browse/PICARD-2355 for this.

Maybe it will be easier to implement a fresh plugins3 without dealing with the legacy code. @rdswift already had collected some ideas quite a while ago, and I have some specific ideas.I'll see I can write this down so we can start a discussion.

phw avatar Jun 12 '23 12:06 phw

Maybe it will be easier to implement a fresh plugins3 without dealing with the legacy code. @rdswift already had collected some ideas quite a while ago, and I have some specific ideas.I'll see I can write this down so we can start a discussion

The notes from my initial discussion can be found at https://github.com/rdswift/picard-plugins/wiki/Picard-Plugins-System-Proposal if you're interested, and the related ticket PICARD-1861.

rdswift avatar Jun 12 '23 15:06 rdswift

Closing, replaced by #2307

phw avatar Sep 09 '23 14:09 phw