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

DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses #334

Closed
mgedmin opened this issue Oct 25, 2019 · 5 comments · Fixed by #478

Comments

@mgedmin
Copy link
Contributor

mgedmin commented Oct 25, 2019

Summary

If you run tox -e py37, one of the things you see is

linkcheck/loader.py:15
  /home/mg/src/linkchecker/linkcheck/loader.py:15: DeprecationWarning: the imp module is deprecated in favour of importlib; see the module's documentation for alternative uses
    import imp

There's only one use of imp: in linkcheck/loader.py's get_folder_modules() :

            yield imp.load_source(modname, fullname)

load_source is documented for Python 2.7, but not for Python 3.5 through 3.8. Was it removed? Who knows.

@mgedmin
Copy link
Contributor Author

mgedmin commented Oct 25, 2019

It was not removed, Python 3 still ships a load_source in imp.py:
https://github.com/python/cpython/blob/96b06aefe23521b61e4e9cdd44f5d30b00c7eb95/Lib/imp.py#L165-L176

Luckily the _LoadSourceCompatibility hack is not needed for us, we could use importlib.machinery.SourceFileLoader(name, pathname) directly, since we don't pass the file argument to load_source.

The use of importlib._bootstrap._load() unnerves me, I'd prefer to avoid calling private methods. I'm sure importlib has a way of loading Python source files from a specific directory on disk without adding that directory to sys.path, without using private methods.

@mgedmin
Copy link
Contributor Author

mgedmin commented Oct 25, 2019

Surprise, surprise, this code is not covered by tests.

@anarcat
Copy link
Contributor

anarcat commented Oct 25, 2019

why do we need this at all?

this code looks super weird to me... we do permission checks, then load_source inside a function called get_folder_modules (hello side effects?)...

why do we have our own plugin system?

in my feed2exec project, i do have a very rudimentary plugin system, but it relies exclusively on Python's packages, nothing special regarding magic directories, search paths or permissions. it just does importlib.import_module on whatever you give it.

I'm not especially proud of this, but it is much simpler. I also realized I didn't need to have "discovery" like we're doing in linkchecker, and I'm not sure why we're doing that in the first place... maybe the GUI needs to be able to list plugins that can be enabled? who knows...

Before embarking in this for feed2exec, I did do a quick tour of other plugin systems. If we do need more elaborate capabilities than just load_module, maybe we could drop more legacy code and reuse an existing plugin system?

I'd probably lean towards whatever PyPA is doing. And of course I wouldn't object to fixing our code either, but I just thought I'd bring up the fundamental design issue here, which is that we have a lot of legacy code in linkchecker, and any time we can flush some of it (like python 2, a custom HTML parser or network engine!) in favor of an existing external package that looks sane, we should. :)

@cjmayo
Copy link
Contributor

cjmayo commented Aug 10, 2020

Updated to use importlib in #478.

The code changed is only supporting an undocumented feature of installing user plugins in ~/.linkchecker/plugins, not the plugins that come with LinkChecker. I did think about removing it, but I guess it could be useful if you are developing a new plugin.

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

Successfully merging a pull request may close this issue.

3 participants