Modify utility code loader as per discussion + tests

Cython/Compiler/Code.py

@@ -44,8 +44,11 @@ uncachable_builtins = [
     'WindowsError',
     ]
 
-Cython_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
-Utility_dir = os.path.join(Cython_dir, "Utility")
+def get_utility_dir():
+    # make this a function and not global variables:
+    # http://trac.cython.org/cython_trac/ticket/475
+    Cython_dir = os.path.dirname(os.path.dirname(os.path.abspath(__file__)))
+    return os.path.join(Cython_dir, "Utility")
 
 class UtilityCodeBase(object):
 
@@ -53,7 +56,7 @@ class UtilityCodeBase(object):
 
     _utility_cache = {}
 
-    @classmethod
+    # @classmethod
     def _add_utility(cls, utility, type, lines, begin_lineno):
         if utility:
             if cls.is_cython_utility:
@@ -68,40 +71,52 @@ class UtilityCodeBase(object):
             else:
                 utility[1] = code
 
-    @classmethod
+    _add_utility = classmethod(_add_utility)
+
+    # @classmethod
     def load_utilities_from_file(cls, path):
         utilities = cls._utility_cache.get(path)
         if utilities:
             return utilities
 
-        filename = os.path.join(Utility_dir, path)
-        f = codecs.open(filename, encoding='UTF-8')
+        filename = os.path.join(get_utility_dir(), path)
 
         _, ext = os.path.splitext(path)
         if ext in ('.pyx', '.py', '.pxd', '.pxi'):
             comment = '#'
         else:
-            comment = '//'
+            comment = '/'
 
-        regex = r'%s\s*Utility(Proto|Code)\s*:\s*((\w|\.)+)\s*' % comment
+        regex = r'%s{5,20}\s*((\w|\.)+)\s*%s{5,20}' % (comment, comment)
         utilities = {}
         lines = []
 
         utility = type = None
         begin_lineno = 0
-        for lineno, line in enumerate(f):
-            m = re.search(regex, line)
-            if m:
-                cls._add_utility(utility, type, lines, begin_lineno)
-
-                begin_lineno = lineno + 1
-                type, name = m.group(1), m.group(2)
-                utility = utilities.setdefault(name, [None, None])
-                utilities[name] = utility
 
-                lines = []
-            else:
-                lines.append(line)
+        f = Utils.open_source_file(filename, encoding='UTF-8')
+        try:
+            for lineno, line in enumerate(f):
+                m = re.search(regex, line)
+                if m:
+                    cls._add_utility(utility, type, lines, begin_lineno)
+
+                    begin_lineno = lineno + 1
+                    name = m.group(1)
+                    if name.endswith(".proto"):
+                        name = name[:-6]
+                        type = 'Proto'
+                    else:
+                        type = 'Code'
+
+                    utility = utilities.setdefault(name, [None, None])
+                    utilities[name] = utility
+
+                    lines = []
+                else:
+                    lines.append(line)
+        finally:
+            f.close()
 
         if not utility:
             raise ValueError("Empty utility code file")
@@ -114,12 +129,15 @@ class UtilityCodeBase(object):
         cls._utility_cache[path] = utilities
         return utilities
 
-    @classmethod
-    def load_utility_from_file(cls, path, util_code_name,
-                               context=None, **kwargs):
+    load_utilities_from_file = classmethod(load_utilities_from_file)
+
+    # @classmethod
+    def load(cls, util_code_name, from_file=None, context=None, **kwargs):
         """
-        Load a utility code from a file specified by path (relative to
-        Cython/Utility) and name util_code_name.
+        Load a utility code from a file specified by from_file (relative to
+        Cython/Utility) and name util_code_name. If from_file is not given,
+        load it from the file util_code_name.*. There should be only one file
+        matched by this pattern.
 
         Utilities in the file can be specified as follows:
 
@@ -137,7 +155,7 @@ class UtilityCodeBase(object):
         one should pass in the 'name' keyword argument to be used for name
         mangling of such entries.
         """
-        proto, impl = cls.load_utility_as_string(path, util_code_name, context)
+        proto, impl = cls.load_as_string(util_code_name, from_file, context)
 
         if proto is not None:
             kwargs['proto'] = proto
@@ -145,36 +163,40 @@ class UtilityCodeBase(object):
             kwargs['impl'] = impl
 
         if 'name' not in kwargs:
-            kwargs['name'] = os.path.splitext(path)[0]
+            if from_file:
+                kwargs['name'] = os.path.splitext(from_file)[0]
+            else:
+                kwargs['name'] = util_code_name
 
         return cls(**kwargs)
 
-    @classmethod
-    def load_utility_as_string(cls, path, util_code_name, context=None):
+    load = classmethod(load)
+
+    # @classmethod
+    def load_as_string(cls, util_code_name, from_file=None, context=None):
         """
         Load a utility code as a string. Returns (proto, implementation)
         """
-        utilities = cls.load_utilities_from_file(path)
+        if from_file is None:
+            files = glob.glob(os.path.join(get_utility_dir(),
+                                           util_code_name + '.*'))
+            if len(files) != 1:
+                raise ValueError("Need exactly one utility file")
+
+            from_file, = files
+
+        utilities = cls.load_utilities_from_file(from_file)
 
         proto, impl = utilities[util_code_name]
-        if proto:
-            if context is not None:
+        if context is not None:
+            if proto:
                 proto = tempita.sub(proto, **context)
-
-        if impl:
-            if context is not None:
+            if impl:
                 impl = tempita.sub(impl, **context)
 
         return proto, impl
 
-    @classmethod
-    def load_utility(cls, name, context=None, **kwargs):
-        "Load utility name with context from a utility file name.suffix"
-        files = glob.glob(os.path.join(Utility_dir, name + '.*'))
-        if len(files) != 1:
-            raise ValueError("Need exactly one utility file")
-
-        return cls.load_utilities_from_file(files[0], name, context, **kwargs)
+    load_as_string = classmethod(load_as_string)
 
     def __str__(self):
         return "<%s(%s)" % (type(self).__name__, self.name)

Cython/Compiler/CythonScope.py

@@ -103,9 +103,8 @@ def create_cython_scope(context, create_testscope):
 
 # Load test utilities for the cython scope
 
-def load_testscope_utility(cython_util_name, *args, **kwargs):
-    return CythonUtilityCode.load_utility_from_file(
-                    "TestCythonScope.pyx", cython_util_name, *args, **kwargs)
+def load_testscope_utility(cy_util_name, **kwargs):
+    return CythonUtilityCode.load(cy_util_name, "TestCythonScope.pyx", **kwargs)
 
 
 undecorated_methods_protos = UtilityCode(proto=u"""

Cython/Compiler/MemoryView.py

@@ -382,8 +382,9 @@ class CopyFuncUtilCode(object):
             copy_contents_name=copy_contents_name
         )
 
-        _, copy_code = UtilityCode.load_utility_as_string(
-                    "MemoryView_C.c", "MemviewSliceCopyTemplate", context)
+        _, copy_code = UtilityCode.load_as_string("MemviewSliceCopyTemplate",
+                                                  from_file="MemoryView_C.c",
+                                                  context=context)
         code.put(copy_code)
 
 
@@ -737,17 +738,11 @@ class MemoryViewSliceTransform(CythonTransform):
         return node
 
 
-def load_memview_cy_utility(name, *args, **kwargs):
-    return CythonUtilityCode.load_utility_from_file(
-                        "MemoryView.pyx", name, *args, **kwargs)
-
-def load_memview_c_utility(name, *args, **kwargs):
-    return UtilityCode.load_utility_from_file(
-                        "MemoryView_C.c", name, *args, **kwargs)
-
-def load_memview_c_string(name):
-    return UtilityCode.load_utility_as_string("MemoryView_C.c", name)
+def load_memview_cy_utility(util_code_name, **kwargs):
+    return CythonUtilityCode.load(util_code_name, "MemoryView.pyx", **kwargs)
 
+def load_memview_c_utility(util_code_name, **kwargs):
+    return UtilityCode.load(util_code_name, "MemoryView_C.c", **kwargs)
 
 context = {
     'memview_struct_name': memview_objstruct_cname,
@@ -755,12 +750,12 @@ context = {
     'memviewslice_name': memviewslice_cname,
 }
 memviewslice_declare_code = load_memview_c_utility(
-        name="MemviewSliceStruct",
+        "MemviewSliceStruct",
         proto_block='utility_code_proto_before_types',
         context=context)
 
 memviewslice_init_code = load_memview_c_utility(
-    name="MemviewSliceInit",
+    "MemviewSliceInit",
     context={'BUF_MAX_NDIMS': Options.buffer_max_dims},
     requires=[memviewslice_declare_code],
 )

Cython/Compiler/Tests/TestUtilityLoad.py

+import unittest
+
+from Cython.Compiler import Code, UtilityCode
+
+
+def strip_2tup(tup):
+    return tup[0] and tup[0].strip(), tup[1] and tup[1].strip()
+
+class TestUtilityLoader(unittest.TestCase):
+    """
+    Test loading UtilityCodes
+    """
+
+    expected = "test {{loader}} prototype", "test {{loader}} impl"
+    expected_tempita = (expected[0].replace('{{loader}}', 'Loader'),
+                        expected[1].replace('{{loader}}', 'Loader'))
+
+    required = "I am a dependency proto", "I am a dependency impl"
+
+    context = dict(loader='Loader')
+
+    name = "TestUtilityLoader"
+    filename = "TestUtilityLoader.c"
+    cls = Code.UtilityCode
+
+    def test_load_as_string(self):
+        got = strip_2tup(self.cls.load_as_string(self.name))
+        self.assertEquals(got, self.expected)
+
+        got = strip_2tup(self.cls.load_as_string(self.name, self.filename))
+        self.assertEquals(got, self.expected)
+
+        got = strip_2tup(self.cls.load_as_string(self.name, context=self.context))
+        self.assertEquals(got, self.expected_tempita)
+
+    def test_load(self):
+        utility = self.cls.load(self.name)
+        got = strip_2tup((utility.proto, utility.impl))
+        self.assertEquals(got, self.expected)
+
+        # Not implemented yet
+        #required, = utility.requires
+        #self.assertEquals((required.proto, required.impl), self.required)
+
+        utility = self.cls.load(self.name, from_file=self.filename)
+        got = strip_2tup((utility.proto, utility.impl))
+        self.assertEquals(got, self.expected)
+
+
+class TestCythonUtilityLoader(TestUtilityLoader):
+    """
+    Test loading CythonUtilityCodes
+    """
+
+    # Just change the attributes and run the same tests
+    expected = None, "test {{cy_loader}} impl"
+    expected_tempita = None, "test CyLoader impl"
+
+    required = None, "I am a Cython dependency impl"
+
+    context = dict(cy_loader='CyLoader')
+
+    name = "TestCyUtilityLoader"
+    filename = "TestCyUtilityLoader.pyx"
+    cls = UtilityCode.CythonUtilityCode
+
+    # Small hack to pass our tests above
+    cls.proto = None
\ No newline at end of file

Cython/Compiler/UtilityCode.py

@@ -70,7 +70,7 @@ class CythonUtilityCode(Code.UtilityCodeBase):
         #    while the generated node trees can be altered in the compilation of a
         #    single file.
         # Hence, delay any processing until later.
-        self.pyx = impl
+        self.impl = impl
         self.name = name
         self.prefix = prefix
         self.requires = requires or []
@@ -86,7 +86,7 @@ class CythonUtilityCode(Code.UtilityCodeBase):
         context = CythonUtilityCodeContext(self.name)
         context.prefix = self.prefix
         #context = StringParseContext(self.name)
-        tree = parse_from_strings(self.name, self.pyx, context=context,
+        tree = parse_from_strings(self.name, self.impl, context=context,
                                   allow_struct_enum_decorator=True)
         pipeline = Pipeline.create_pipeline(context, 'pyx', exclude_classes=excludes)
 

Cython/Utility/MemoryView.pyx

-# UtilityCode: CythonArray
+########## CythonArray ##########
 
 cdef extern from "stdlib.h":
     void *malloc(size_t)
@@ -134,7 +134,7 @@ cdef class array:
 cdef array array_cwrapper(tuple shape, Py_ssize_t itemsize, char *format, char *mode):
     return array(shape, itemsize, format, mode.decode('ASCII'))
 
-# UtilityCode: MemoryView
+########## MemoryView ##########
 
 # from cpython cimport ...
 cdef extern from "pythread.h":

Cython/Utility/MemoryView_C.c

-// UtilityProto: MemviewSliceStruct
+////////// MemviewSliceStruct.proto //////////
 
 /* memoryview slice struct */
 
@@ -11,7 +11,7 @@ typedef struct {
   Py_ssize_t suboffsets[{{max_dims}}];
 } {{memviewslice_name}};
 
-// UtilityProto: MemviewSliceInit
+////////// MemviewSliceInit.proto //////////
 
 #define __Pyx_BUF_MAX_NDIMS %(BUF_MAX_NDIMS)d
 
@@ -36,7 +36,7 @@ static int __Pyx_init_memviewslice(
                 int ndim,
                 __Pyx_memviewslice *memviewslice);
 
-// UtilityCode: MemviewSliceInit
+////////// MemviewSliceInit //////////
 
 static int __Pyx_ValidateAndInit_memviewslice(
                 struct __pyx_memoryview_obj *memview,
@@ -218,7 +218,7 @@ no_fail:
     return retval;
 }
 
-// UtilityCode: MemviewSliceCopyTemplate
+////////// MemviewSliceCopyTemplate //////////
 
 static __Pyx_memviewslice {{copy_name}}(const __Pyx_memviewslice from_mvs) {
 

Cython/Utility/TestCyUtilityLoader.pyx

+########## TestCyUtilityLoader ##########
+test {{cy_loader}} impl

Cython/Utility/TestCythonScope.pyx

-# UtilityCode: TestClass
+########## TestClass ##########
 # These utilities are for testing purposes
 
 cdef extern from *:
@@ -44,19 +44,19 @@ cdef test_call(obj):
 cdef _testclass_new(int value):
     return TestClass(value)
 
-# UtilityCode: TestDep
+########### TestDep ##########
 
 @cname('__pyx_test_dep')
 cdef test_dep(obj):
     print 'test_dep', obj
 
-# UtilityCode: TestScope
+########## TestScope ##########
 
 @cname('__pyx_testscope')
 cdef object _testscope(int value):
     return "hello from cython scope, value=%d" % value
 
-# UtilityCode: View.TestScope
+########## View.TestScope ##########
 
 @cname('__pyx_view_testscope')
 cdef object _testscope(int value):

Cython/Utility/TestUtilityLoader.c

+////////// TestUtilityLoader.proto //////////
+test {{loader}} prototype
+
+////////// TestUtilityLoader //////////
+test {{loader}} impl

MANIFEST.in

@@ -18,7 +18,7 @@ include Demos/freeze/*
 include Demos/libraries/*
 include Demos/Makefile*
 recursive-include Cython/Debugger/Tests *.pyx *.pxd *.c *.h
-recursive-include Cython/Utility *
+recursive-include Cython/Utility *.pyx *.pxd *.c *.h
 recursive-include Tools *
 recursive-include tests *.pyx *.pxd *.pxi *.py *.h *.BROKEN bugs.txt
 recursive-include tests *_lib.cpp *.srctree

runtests.py

@@ -1156,7 +1156,11 @@ class TagsSelector:
 class RegExSelector:
     
     def __init__(self, pattern_string):
-        self.pattern = re.compile(pattern_string, re.I|re.U)
+        try:
+            self.pattern = re.compile(pattern_string, re.I|re.U)
+        except re.error:
+            print('Invalid pattern: %r' % pattern_string)
+            raise
 
     def __call__(self, testname, tags=None):
         return self.pattern.search(testname)