-
Notifications
You must be signed in to change notification settings - Fork 533
fix: python 2 Function interfaces recompatibility #2093
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
Changes from 7 commits
18edf55
2602952
179ed74
402e2f8
18edfe3
97fc8c2
16992d8
1d54a0b
f493bbd
8fd24ea
d3ff652
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 |
---|---|---|
@@ -0,0 +1,47 @@ | ||
# -*- coding: utf-8 -*- | ||
""" | ||
Handles custom functions used in Function interface. Future imports | ||
are avoided to keep namespace as clear as possible. | ||
""" | ||
from builtins import next, str | ||
from future.utils import raise_from | ||
import inspect | ||
from textwrap import dedent | ||
|
||
def getsource(function): | ||
"""Returns the source code of a function""" | ||
return dedent(inspect.getsource(function)) | ||
|
||
|
||
def create_function_from_source(function_source, imports=None): | ||
"""Return a function object from a function source | ||
|
||
Parameters | ||
---------- | ||
function_source : unicode string | ||
unicode string defining a function | ||
imports : list of strings | ||
list of import statements in string form that allow the function | ||
to be executed in an otherwise empty namespace | ||
""" | ||
ns = {} | ||
import_keys = [] | ||
|
||
try: | ||
if imports is not None: | ||
for statement in imports: | ||
exec(statement, ns) | ||
import_keys = list(ns.keys()) | ||
exec(function_source, ns) | ||
|
||
except Exception as e: | ||
msg = 'Error executing function\n{}\n'.format(function_source) | ||
msg += ("Functions in connection strings have to be standalone. " | ||
"They cannot be declared either interactively or inside " | ||
"another function or inline in the connect string. Any " | ||
"imports should be done inside the function.") | ||
raise_from(RuntimeError(msg), e) | ||
ns_funcs = list(set(ns) - set(import_keys + ['__builtins__'])) | ||
assert len(ns_funcs) == 1, "Function or inputs are ill-defined" | ||
func = ns[ns_funcs[0]] | ||
return func |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
# -*- coding: utf-8 -*- | ||
import sys | ||
import pytest | ||
from nipype.utils.functions import (getsource, create_function_from_source) | ||
|
||
def _func1(x): | ||
return x**3 | ||
|
||
def test_func_to_str(): | ||
|
||
def func1(x): | ||
return x**2 | ||
|
||
# Should be ok with both functions! | ||
for f in _func1, func1: | ||
f_src = getsource(f) | ||
f_recreated = create_function_from_source(f_src) | ||
assert f(2.3) == f_recreated(2.3) | ||
|
||
def test_func_to_str_err(): | ||
bad_src = "obbledygobbledygook" | ||
with pytest.raises(RuntimeError): create_function_from_source(bad_src) | ||
|
||
@pytest.mark.skipif(sys.version_info[0] > 2, reason="breaks python 3") | ||
def test_func_py2(): | ||
def is_string(): | ||
return isinstance('string', str) | ||
|
||
def print_statement(): | ||
# test python 2 compatibility | ||
exec('print ""') | ||
|
||
wrapped_func = create_function_from_source(getsource(is_string)) | ||
assert is_string() == wrapped_func() | ||
|
||
wrapped_func2 = create_function_from_source(getsource(print_statement)) | ||
wrapped_func2() | ||
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. no test here? 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. will raise syntax error if print can't be done - is there a better way to do this? 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. with pytest.raises(SyntaxError): wrapped_func2() 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. ah but it doesn't error by default. so you should return something from the function call and check with assert. |
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.
why would this function break python 3?
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.
perhaps separate out into two tests - the print statement which breaks python 3 and the function which should be fine for both.
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.
@satra it'll raise an
AssertionError
- I've isolated the tests though