Commit 99b525a1 authored by Iliya Manolov's avatar Iliya Manolov Committed by Ivan Tyagov

Fixed multiple import errors in Jupyter

Copypasta from the other MR [here](nexedi/erp5!230 (comment 22890)). I found a problem and tried again from scratch. I will close that Merge Request once this one has been approved...



@Tyagov
Currently, when running a Jupyter notebook using some types of imports leads to errors. With this fix the following ways to import things are working:
```python
import string                  # worked before fix
import string as s             # worked before fix
from string import ascii_lowercase             # worked before fix
from string import ascii_lowercase, ascii_uppercase, digits           # fixed - used to import only the first thing
from string import ascii_lowercase as a, ascii_uppercase as b         # fixed - used to give "Error at Server Side"
from string import *                                                  # fixed - used to give "Error at Server Side"
from string import Template                                           # works
```
This was happening because after executing every cell the code would move between SlapOS nodes and lose the imported modules/classes/stuff. This was partially fixed before, but this fix should cover all use cases. I have also added tests for these cases in testExecuteJupyter...

/reviewed-on nexedi/erp5!233
parent 2f9c1d13
...@@ -15,6 +15,7 @@ import json ...@@ -15,6 +15,7 @@ import json
import transaction import transaction
import Acquisition import Acquisition
import astor import astor
import importlib
from Products.ERP5Type.Log import log from Products.ERP5Type.Log import log
def Base_executeJupyter(self, python_expression=None, reference=None, \ def Base_executeJupyter(self, python_expression=None, reference=None, \
...@@ -677,38 +678,101 @@ class ImportFixer(ast.NodeTransformer): ...@@ -677,38 +678,101 @@ class ImportFixer(ast.NodeTransformer):
and environment function which setups the module and return it to be merged and environment function which setups the module and return it to be merged
with the user context. with the user context.
""" """
module_name = node.names[0].name
test_import_string = None test_import_string = None
result_name = ""
root_module_name = ""
module_names = []
if getattr(node, "module", None) is not None: if getattr(node, "module", None) is not None:
# case when 'from <module_name> import <something>' # case when 'from <module_name> import <something>'
test_import_string = "from %s import %s" %(node.module, module_name) root_module_name = node.module
# XXX: handle sub case when "from <module_name> import *"
#if module_name == '*': if (node.names[0].name == '*'):
# module_name = '%s_ALL' %(node.module) # case when "from <module_name> import *"
#else: mod = importlib.import_module(node.module)
# module_name = '%s_%s' %(node.module, module_name) tmp_dict = mod.__dict__
if getattr(node.names[0], 'asname'): for name in tmp_dict.keys():
# case when 'import <module_name> as <name>' if (name[0] != '_'):
module_name = node.names[0].asname module_names.append(name)
test_import_string = "import %s as %s" %(node.names[0].name, module_name)
test_import_string = "from %s import *" %(node.module)
if test_import_string is None: result_name = "%s_ALL" %(node.module)
# case 'import <module_name> else:
# case when "from <module_name> import a as b, c as d, ..."
original_names = []
as_names = []
for name in node.names:
original_names.append(name.name)
if getattr(name, "asname", None) is None:
as_names.append(None)
else:
as_names.append(name.asname)
test_import_string = "from %s import " %(node.module)
for i in range(0, len(original_names)):
test_import_string = test_import_string + original_names[i]
if as_names[i]!=None:
test_import_string = test_import_string + ' as %s' %(as_names[i])
test_import_string = test_import_string + ', '
test_import_string = test_import_string[:-2]
module_names = []
for i in range(0, len(original_names)):
if as_names[i]!=None:
module_names.append(as_names[i])
else:
module_names.append(original_names[i])
for i in range(0, len(original_names)):
if as_names[i]!=None:
result_name = result_name + '%s_' %(as_names[i])
else:
result_name = result_name + '%s_' %(original_names[i])
result_name = result_name[:-1]
elif getattr(node.names[0], 'asname'):
# case when "import <module_name> as <name>""
module_names = [(node.names[0].asname), ]
test_import_string = "import %s as %s" %(node.names[0].name,
module_names[0])
result_name = node.names[0].asname
root_module_name = node.names[0].name
else:
# case when "import <module_name>"
module_names = [(node.names[0].name), ]
test_import_string = "import %s" %node.names[0].name test_import_string = "import %s" %node.names[0].name
result_name = node.names[0].name
root_module_name = node.names[0].name
#log('%s : %s' %(module_name, test_import_string)) final_module_names = []
if not self.import_func_dict.get(module_name): for name in module_names:
if not self.import_func_dict.get(name):
final_module_names.append(name)
log("module_names[0]: " + module_names[0])
log("result_name: " + result_name)
if final_module_names:
# try to import module before it is added to environment # try to import module before it is added to environment
# this way if user tries to import non existent module Exception # this way if user tries to import non existent module Exception
# is immediately raised and doesn't block next Jupyter cell execution # is immediately raised and doesn't block next Jupyter cell execution
exec(test_import_string) exec(test_import_string)
empty_function = self.newEmptyFunction("%s_setup" % module_name)
return_dict = self.newReturnDict(module_name) empty_function = self.newEmptyFunction("%s_setup" %result_name)
return_dict = self.newReturnDict(final_module_names)
log(return_dict)
empty_function.body = [node, return_dict] empty_function.body = [node, return_dict]
environment_set = self.newEnvironmentSetCall("%s_setup" % module_name) environment_set = self.newEnvironmentSetCall("%s_setup" %result_name)
warning = self.newImportWarningCall(module_name) warning = self.newImportWarningCall(root_module_name, result_name)
return [empty_function, environment_set, warning] return [empty_function, environment_set, warning]
else: else:
return node return node
...@@ -721,13 +785,16 @@ class ImportFixer(ast.NodeTransformer): ...@@ -721,13 +785,16 @@ class ImportFixer(ast.NodeTransformer):
func_body = "def %s(): pass" % func_name func_body = "def %s(): pass" % func_name
return ast.parse(func_body).body[0] return ast.parse(func_body).body[0]
def newReturnDict(self, module_name): def newReturnDict(self, module_names):
""" """
Return an AST.Expr representing a returned dict with one single key named Return an AST.Expr representing a returned dict with one single key named
`'module_name'` (as string) which returns the variable `module_name` (as `'module_name'` (as string) which returns the variable `module_name` (as
expression). expression).
""" """
return_dict = "return {'%s': %s}" % (module_name, module_name) return_dict = "return {"
for name in module_names:
return_dict = return_dict + "'%s': %s, " % (name, name)
return_dict = return_dict + '}'
return ast.parse(return_dict).body[0] return ast.parse(return_dict).body[0]
def newEnvironmentSetCall(self, func_name): def newEnvironmentSetCall(self, func_name):
...@@ -739,18 +806,18 @@ class ImportFixer(ast.NodeTransformer): ...@@ -739,18 +806,18 @@ class ImportFixer(ast.NodeTransformer):
tree = ast.parse(code_string) tree = ast.parse(code_string)
return tree.body[0] return tree.body[0]
def newImportWarningCall(self, module_name): def newImportWarningCall(self, module_name, function_name):
""" """
Return an AST.Expr representanting a print statement with a warning to an Return an AST.Expr representanting a print statement with a warning to an
user about the import of a module named `module_name` and instructs him user about the import of a module named `module_name` and instructs him
on how to fix it. on how to fix it.
""" """
warning = ("print '" warning = ("print '"
"WARNING: Your imported the module %s without using " "WARNING: Your imported from the module %s without "
"the environment object, which is not recomended. " "using the environment object, which is not recomended. "
"Your import was automatically converted to use such method." "Your import was automatically converted to use such method."
"The setup function was named as: %s_setup.\\n" "The setup function was named as: %s_setup.\\n"
"'") % (module_name, module_name) "'") % (module_name, function_name)
tree = ast.parse(warning) tree = ast.parse(warning)
return tree.body[0] return tree.body[0]
......
...@@ -735,3 +735,124 @@ context.Base_renderAsHtml(iframe) ...@@ -735,3 +735,124 @@ context.Base_renderAsHtml(iframe)
self.assertTrue(pivottable_frame_display_path in json_result['code_result']) self.assertTrue(pivottable_frame_display_path in json_result['code_result'])
def testConsecutiveImports(self):
'''
This test guarantees that importing a module's variables consecutively in
Jupyter works.
'''
self.login('dev_user')
import_code = '''
from string import ascii_lowercase, ascii_uppercase, digits
'''
reference = 'Test.Notebook.EnvironmentObject.Errors.Import'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=import_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
jupyter_code = '''
print ascii_lowercase
'''
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=jupyter_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), 'abcdefghijklmnopqrstuvwxyz')
jupyter_code = '''
print ascii_uppercase
'''
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=jupyter_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), 'ABCDEFGHIJKLMNOPQRSTUVWXYZ')
jupyter_code = '''
print digits
'''
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=jupyter_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), '0123456789')
def testStarImport(self):
'''
This test guarantees that "from x import *" works in Jupyter.
'''
self.login('dev_user')
import_code = '''
from string import *
'''
reference = 'Test.Notebook.EnvironmentObject.Errors.Import'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=import_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
jupyter_code = '''
print ascii_lowercase
'''
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=jupyter_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), 'abcdefghijklmnopqrstuvwxyz')
def testAsImport(self):
'''
This test guarantees that "from x import a as b" works in Jupyter.
'''
self.login('dev_user')
import_code = '''
from string import digits as dig
'''
reference = 'Test.Notebook.EnvironmentObject.Errors.Import'
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=import_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
jupyter_code = '''
print dig
'''
result = self.portal.Base_executeJupyter(
reference=reference,
python_expression=jupyter_code
)
self.tic()
result = json.loads(result)
self.assertEquals(result['status'], 'ok')
self.assertEquals(result['code_result'].strip(), '0123456789')
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment