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

Deprecate Use of imp in Favor of importlib on Python >= 3.1 #386

Open
derks opened this Issue Jul 7, 2016 · 5 comments

Comments

Projects
None yet
3 participants
@derks
Member

derks commented Jul 7, 2016

The imp module is deprecated in favor of importlib as of Python 3.4 ... should update Cement (with backward compat for imp on Python < 3.1).

@derks derks added the stable/2.10.x label Jul 7, 2016

@derks derks added this to the 2.10.0 Stable milestone Jul 7, 2016

@derks derks self-assigned this Jul 7, 2016

@akhilman

This comment has been minimized.

Show comment
Hide comment
@akhilman

akhilman Jul 8, 2016

Contributor

I have a problem with importlib.

We have a structure:

./test_imp.py
./test_importlib.py
./plug/foo/__init__.py
./plug/foo/bar.py

./plug/foo/__init__.py

from .bar import baz

./plug/foo/bar.py

baz = 1

./test_imp.py

import imp

# NOTE we do not need to specify file path and all that suffixes
f, path, desc = imp.find_module('foo', ['./plug'])
mod = imp.load_module('foo', f, path, desc)
print(mod.baz)

./test_importlib.py

import importlib
import importlib.util

spec = importlib.util.spec_from_file_location('foo', './plug/foo/__init__.py')
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
print(mod.baz)

imp example works well but importlib raises with

Traceback (most recent call last):
  File "test_miportlib.py", line 6, in <module>
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 665, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "./plug/foo/__init__.py", line 1, in <module>
    from .bar import baz
SystemError: Parent module 'foo' not loaded, cannot perform relative import

This might be related to https://bugs.python.org/issue18018
I propose to use imp until we get relative import work.

Contributor

akhilman commented Jul 8, 2016

I have a problem with importlib.

We have a structure:

./test_imp.py
./test_importlib.py
./plug/foo/__init__.py
./plug/foo/bar.py

./plug/foo/__init__.py

from .bar import baz

./plug/foo/bar.py

baz = 1

./test_imp.py

import imp

# NOTE we do not need to specify file path and all that suffixes
f, path, desc = imp.find_module('foo', ['./plug'])
mod = imp.load_module('foo', f, path, desc)
print(mod.baz)

./test_importlib.py

import importlib
import importlib.util

spec = importlib.util.spec_from_file_location('foo', './plug/foo/__init__.py')
mod = importlib.util.module_from_spec(spec)
spec.loader.exec_module(mod)
print(mod.baz)

imp example works well but importlib raises with

Traceback (most recent call last):
  File "test_miportlib.py", line 6, in <module>
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 665, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "./plug/foo/__init__.py", line 1, in <module>
    from .bar import baz
SystemError: Parent module 'foo' not loaded, cannot perform relative import

This might be related to https://bugs.python.org/issue18018
I propose to use imp until we get relative import work.

@derks derks added 1 - Ready and removed 1 - Ready labels Jul 12, 2016

derks added a commit that referenced this issue Jul 12, 2016

@derks derks added 1 - Ready and removed 2 - Working <= 5 labels Jul 12, 2016

@derks derks removed this from the 2.10.0 Stable milestone Jul 13, 2016

derks added a commit that referenced this issue Nov 23, 2016

@derks derks added this to the 2.12.0 Stable milestone Nov 23, 2016

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Nov 23, 2016

Member

@akhilman I've completed the implementation of deprecating imp in favor of importlib, but as you've outlined this is blocked due to the fact that we can't do relative imports inside directory based plugins.

Unfortunately the Python Bug #18018 only relates to the fact that the situation currently raises a SystemError exception, where it should be an ImportError. I don't see this as a bug in Python per-say, but rather that we're trying to use it in an unintended manner.

Wondering if anyone might have any futher insights?

Member

derks commented Nov 23, 2016

@akhilman I've completed the implementation of deprecating imp in favor of importlib, but as you've outlined this is blocked due to the fact that we can't do relative imports inside directory based plugins.

Unfortunately the Python Bug #18018 only relates to the fact that the situation currently raises a SystemError exception, where it should be an ImportError. I don't see this as a bug in Python per-say, but rather that we're trying to use it in an unintended manner.

Wondering if anyone might have any futher insights?

@derks derks added dev/2.11.x and removed stable/2.10.x labels Nov 23, 2016

derks added a commit that referenced this issue Nov 23, 2016

@derks

This comment has been minimized.

Show comment
Hide comment
@derks

derks Nov 23, 2016

Member

Anyone interested in working this issue can checkout the issue386 branch, and run:

$ python setup.py nosetests -a plugin

======================================================================
ERROR: test_plugins (tests.ext.plugin_tests.PluginExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/derks/Development/cement/tests/ext/plugin_tests.py", line 23, in test_plugins
    with self.app as app:
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 1494, in __enter__
    self.setup()
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 852, in setup
    self._setup_plugin_handler()
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 1332, in _setup_plugin_handler
    self.plugin.load_plugins(self.plugin.get_enabled_plugins())
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 327, in load_plugins
    self.load_plugin(plugin_name)
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 290, in load_plugin
    if self._load_plugin_from_dir(plugin_name, load_dir):
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 227, in _load_plugin_from_dir
    return import_func(plugin_name, plugin_dir)
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 192, in _load_plugin_via_importlib
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 662, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/Users/derks/Development/cement/tests/__data__/plugins/plugin2/__init__.py", line 2, in <module>
    from .foo import bar
SystemError: Parent module 'plugin2' not loaded, cannot perform relative import

Goal is to get tests.ext.plugin_tests.PluginExtTestCase test to pass (loading a directory based plugin with relative imports.

Member

derks commented Nov 23, 2016

Anyone interested in working this issue can checkout the issue386 branch, and run:

$ python setup.py nosetests -a plugin

======================================================================
ERROR: test_plugins (tests.ext.plugin_tests.PluginExtTestCase)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "/Users/derks/Development/cement/tests/ext/plugin_tests.py", line 23, in test_plugins
    with self.app as app:
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 1494, in __enter__
    self.setup()
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 852, in setup
    self._setup_plugin_handler()
  File "/Users/derks/Development/cement/cement/core/foundation.py", line 1332, in _setup_plugin_handler
    self.plugin.load_plugins(self.plugin.get_enabled_plugins())
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 327, in load_plugins
    self.load_plugin(plugin_name)
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 290, in load_plugin
    if self._load_plugin_from_dir(plugin_name, load_dir):
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 227, in _load_plugin_from_dir
    return import_func(plugin_name, plugin_dir)
  File "/Users/derks/Development/cement/cement/ext/ext_plugin.py", line 192, in _load_plugin_via_importlib
    spec.loader.exec_module(mod)
  File "<frozen importlib._bootstrap_external>", line 662, in exec_module
  File "<frozen importlib._bootstrap>", line 222, in _call_with_frames_removed
  File "/Users/derks/Development/cement/tests/__data__/plugins/plugin2/__init__.py", line 2, in <module>
    from .foo import bar
SystemError: Parent module 'plugin2' not loaded, cannot perform relative import

Goal is to get tests.ext.plugin_tests.PluginExtTestCase test to pass (loading a directory based plugin with relative imports.

@akhilman

This comment has been minimized.

Show comment
Hide comment
@akhilman

akhilman Jan 10, 2017

Contributor

submodule_search_locations in ModuleSpec looks interesting.
I would like to try to set this attribute to importing module directory name.

Contributor

akhilman commented Jan 10, 2017

submodule_search_locations in ModuleSpec looks interesting.
I would like to try to set this attribute to importing module directory name.

@grbd

This comment has been minimized.

Show comment
Hide comment
@grbd

grbd Jun 27, 2017

Hi,
I recently stumbled across this issue when googling for a solution for another project with a similar problem. I managed to figure a way of importing by looking at the code for importlib

# New way of importing
def path_import2(absolute_path):
   spec = importlib.util.spec_from_file_location(absolute_path, absolute_path)
   module = spec.loader.load_module(spec.name)
   return module

# Original way of importing
def path_import1(absolute_path):
   '''implementation taken from https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly'''
   spec = importlib.util.spec_from_file_location(absolute_path, absolute_path)
   module = importlib.util.module_from_spec(spec)
   spec.loader.exec_module(module)
   return module

grbd commented Jun 27, 2017

Hi,
I recently stumbled across this issue when googling for a solution for another project with a similar problem. I managed to figure a way of importing by looking at the code for importlib

# New way of importing
def path_import2(absolute_path):
   spec = importlib.util.spec_from_file_location(absolute_path, absolute_path)
   module = spec.loader.load_module(spec.name)
   return module

# Original way of importing
def path_import1(absolute_path):
   '''implementation taken from https://docs.python.org/3/library/importlib.html#importing-a-source-file-directly'''
   spec = importlib.util.spec_from_file_location(absolute_path, absolute_path)
   module = importlib.util.module_from_spec(spec)
   spec.loader.exec_module(module)
   return module

@derks derks removed the 2 - Working <= 5 label Jan 17, 2018

@derks derks removed this from the 2.12.0 Stable milestone Apr 13, 2018

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