-
Notifications
You must be signed in to change notification settings - Fork 167
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
Stop using the deprecated imp module when possible #208
Changes from 6 commits
78d4d7a
a4315c3
afc3aeb
bceef22
20fed6b
2508e9e
4ab821c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ | |
|
||
import dis | ||
from functools import partial | ||
import imp | ||
import importlib | ||
import io | ||
import itertools | ||
import logging | ||
|
@@ -304,20 +304,10 @@ def save_module(self, obj): | |
""" | ||
Save a module as an import | ||
""" | ||
mod_name = obj.__name__ | ||
# If module is successfully found then it is not a dynamically created module | ||
if hasattr(obj, '__file__'): | ||
is_dynamic = False | ||
else: | ||
try: | ||
_find_module(mod_name) | ||
is_dynamic = False | ||
except ImportError: | ||
is_dynamic = True | ||
|
||
self.modules.add(obj) | ||
if is_dynamic: | ||
self.save_reduce(dynamic_subimport, (obj.__name__, vars(obj)), obj=obj) | ||
if _is_dynamic(obj): | ||
self.save_reduce(dynamic_subimport, (obj.__name__, vars(obj)), | ||
obj=obj) | ||
else: | ||
self.save_reduce(subimport, (obj.__name__,), obj=obj) | ||
|
||
|
@@ -949,7 +939,7 @@ def subimport(name): | |
|
||
|
||
def dynamic_subimport(name, vars): | ||
mod = imp.new_module(name) | ||
mod = types.ModuleType(name) | ||
mod.__dict__.update(vars) | ||
return mod | ||
|
||
|
@@ -1146,19 +1136,31 @@ def _rehydrate_skeleton_class(skeleton_class, class_dict): | |
return skeleton_class | ||
|
||
|
||
def _find_module(mod_name): | ||
def _is_dynamic(module): | ||
""" | ||
Iterate over each part instead of calling imp.find_module directly. | ||
This function is able to find submodules (e.g. scikit.tree) | ||
Return True if the module is special module that cannot be imported by its | ||
name. | ||
""" | ||
path = None | ||
for part in mod_name.split('.'): | ||
if path is not None: | ||
path = [path] | ||
file, path, description = imp.find_module(part, path) | ||
if file is not None: | ||
file.close() | ||
return path, description | ||
# Quick check: module that have __file__ attribute are not dynamic modules. | ||
if hasattr(module, '__file__'): | ||
return False | ||
|
||
if hasattr(module, '__spec__'): | ||
return module.__spec__ is None | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice one!! This is indeed better There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The old There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In particular, I cannot build a valid case that would make it possible to have perfect coverage of the I don't think it was covered either before. |
||
else: | ||
# Backward compat for Python 2 | ||
import imp | ||
try: | ||
path = None | ||
for part in module.__name__.split('.'): | ||
if path is not None: | ||
path = [path] | ||
f, path, description = imp.find_module(part, path) | ||
if f is not None: | ||
f.close() | ||
except ImportError: | ||
return True | ||
return False | ||
|
||
|
||
"""Constructors for 3rd party libraries | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This line, in particular, was never covered by past tests.