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

Improve non-functional Cython error messages #402

Merged
merged 3 commits into from Feb 20, 2019
Merged
Changes from 1 commit
Commits
File filter...
Filter file types
Jump to…
Jump to file or symbol
Failed to load files and symbols.

Always

Just for now

@@ -403,36 +403,42 @@ def _test_inline(cls):
extra_compile_args=extra_compile_args)
cls._use_inline = True
except (weave.build_tools.CompileError,
distutils.errors.CompileError, ImportError):
pass
except ValueError as e:
if len(e.args) == 1 and \
e.args[0] == "Symbol table not found":
get_logger(__name__).debug(
"'ValueError: Symbol table not found' "
"encountered; weave compiler is not functional")
else:
distutils.errors.CompileError,
ImportError,
ValueError) as e:
if isinstance(e, ValueError) and (

This comment has been minimized.

Copy link
@jmuhlich

jmuhlich Feb 5, 2019

Member

This logic was already a little obscure and it's duplicated here and below. The immediate debug message previously made it fairly clear what was being checked, but with this change it's less apparent what's going on. Maybe this is a good time to refactor it. Could you refactor it into a well-named predicate function on e with an explanation of what it's looking for? Also I don't think rewriting the arg-checking term as a disjunction of negations made it any better... I do prefer the old form. Or maybe just e.args == ('Symbol table not found',) which actually covers the whole thing at once?

len(e.args) != 1 or
e.args[0] != 'Symbol table not found'):
raise
logger.warn('weave compiler appears non-functional. {}\n'
'Original error: {}'.format(
_c_compiler_message(), repr(e)))

@classmethod
def _test_cython(cls):
if not hasattr(cls, '_use_cython'):
logger = get_logger(__name__)
cls._use_cython = False
if Cython is None:
return
try:
Cython.inline('x = 1', force=True, quiet=True)
cls._use_cython = True
except Cython.Compiler.Errors.CompileError:
pass
except ValueError as e:
if len(e.args) == 1 and e.args[0] == "Symbol table not found":
get_logger(__name__).debug(
"'ValueError: Symbol table not found' "
"encountered; Cython compiler is not functional")
else:
except (Cython.Compiler.Errors.CompileError,
distutils.errors.DistutilsPlatformError,
ValueError) as e:
if isinstance(e, distutils.errors.DistutilsPlatformError) and \
str(e) != 'Unable to find vcvarsall.bat':

This comment has been minimized.

Copy link
@jmuhlich

jmuhlich Feb 5, 2019

Member

Does this error not also occur under weave.inline? Can we unify all the C compiler breakage handling into one place?

raise
if isinstance(e, ValueError) and (
len(e.args) != 1 or
e.args[0] != 'Symbol table not found'):
raise

logger.warn('Cython compiler appears non-functional. {}\n'
'Original error: {}'.format(
_c_compiler_message(), repr(e)))

@classmethod
def _autoselect_compiler(cls):
""" Auto-select equation backend """
@@ -592,3 +598,14 @@ def process(self, msg, kwargs):
warn = logging.LoggerAdapter.info
# Provide 'fatal' to match up with distutils log functions.
fatal = logging.LoggerAdapter.critical


def _c_compiler_message():
message = 'Please check you have a functional C compiler'
if os.name == 'nt':
message += ', available from ' \
'https://wiki.python.org/moin/WindowsCompilers'
else:
message += '.'

return message
ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.