Skip to content

BF: be resilient to optional module non-ImportError exceptions upon import #618

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.

Sign up for GitHub

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

Merged
merged 3 commits into from
Apr 12, 2018
Merged

BF: be resilient to optional module non-ImportError exceptions upon import #618

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
45a6e87
BF: be resilient to optional module non-ImportError exceptions upon i…
yarikoptic Apr 11, 2018
f80e5ce
BF(PY3): just catch all Exceptions, patch builtins module now provide…
yarikoptic Apr 12, 2018
2c51018
BF: flake8 the import
yarikoptic Apr 12, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions nibabel/optpkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,10 +94,14 @@ def optional_package(name, trip_msg=None, min_version=None):
# fromlist=[''] results in submodule being returned, rather than the top
# level module. See help(__import__)
fromlist = [''] if '.' in name else []
exc = None
try:
pkg = __import__(name, fromlist=fromlist)
except ImportError:
pass
except Exception as exc_:
# Could fail due to some ImportError or for some other reason
# e.g. h5py might have been checking file system to support UTF-8
# etc. We should not blow if they blow
exc = exc_ # So it is accessible outside of the code block
else: # import worked
# top level module
if check_version(pkg):
Expand All @@ -111,8 +115,8 @@ def optional_package(name, trip_msg=None, min_version=None):
(name, min_version))
if trip_msg is None:
trip_msg = ('We need package %s for these functions, but '
'``import %s`` raised an ImportError'
% (name, name))
'``import %s`` raised %s'
% (name, name, exc))
pkg = TripWire(trip_msg)

def setup_module():
Expand Down
3 changes: 3 additions & 0 deletions nibabel/py3k.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ def open_latin1(filename, mode='r'):
ints2bytes = lambda seq: bytes(seq)
ZEROB = bytes([0])
FileNotFoundError = FileNotFoundError
import builtins
else:
import StringIO
StringIO = BytesIO = StringIO.StringIO
Expand All @@ -66,6 +67,8 @@ def open_latin1(filename, mode='r'):
class FileNotFoundError(IOError):
pass

import __builtin__ as builtins # flake8: noqa F401


def getexception():
return sys.exc_info()[1]
Expand Down
8 changes: 8 additions & 0 deletions nibabel/tests/test_optpkg.py
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
""" Testing optpkg module
"""

import mock
import types
import sys
from distutils.version import LooseVersion
Expand All @@ -10,6 +11,7 @@
assert_equal)


from nibabel.py3k import builtins
from nibabel.optpkg import optional_package
from nibabel.tripwire import TripWire, TripWireError

Expand All @@ -36,6 +38,12 @@ def test_basic():
assert_good('os.path')
# We never have package _not_a_package
assert_bad('_not_a_package')
def raise_Exception(*args, **kwargs):
raise Exception(
"non ImportError could be thrown by some malfunctioning module "
"upon import, and optional_package should catch it too")
with mock.patch.object(builtins, '__import__', side_effect=raise_Exception):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Python 3, builtins is __builtin__, and in Python 2 you're setting builtins = __builtin__. Could you just import __builtin__ and patch that here? I'm not sure what we're getting by adding this to py3k.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • In Python 3, builtins is __builtin__ . I am confused with what you mean ;):
$> python3 -c 'import builtins'      
$> python3 -c 'import __builtin__'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
ModuleNotFoundError: No module named '__builtin__'
  • py3k is a module to establish the compatibility layer and feels intended to do so that code using it stays "python3" like. With this change then any code which needs to use builtins module in either of pythons would do so with python3 naming. I didn't want to breed python 2-vs-3 code right here in the test

Copy link
Member

@effigies effigies Apr 12, 2018 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, perhaps this is an IPython thing.

In [1]: __builtin__
Out[1]: <module 'builtins' (built-in)>

But:

$ python -c 'import builtins; print(builtins is __builtin__)'
Traceback (most recent call last):
  File "<string>", line 1, in <module>
NameError: name '__builtin__' is not defined

assert_bad('nottriedbefore')


def test_versions():
Expand Down