[MERGE] import both c-extension and python module with the same name for testing

Martin Pool mbp at canonical.com
Tue Aug 7 05:41:21 BST 2007


Martin Pool has voted resubmit.
Status is now: Resubmit
Comment:
This sounds very nice to me and I like the way you tested it.

+    def feature_name(self):
+        return 'bzrlib.tests.dummy compiled extension'
+
+DummyPyrexFeature = _DummyPyrexFeature()

One more blank line after the class please.

+"""Testing both python module and pyrex implemention
+of dummy module
+"""
+

The first line of the docstring should be just one line.
(I realize there are some others where this is not true.)

+def load_py_and_c(modulename):
+    """Try to import both compiled extension and python version
+    of module with modulename.
+
+    :param modulename:  (dotted) module name to import
+    :return:    (python module, c-extension module)
+    :raises:    ImportError if no python module found
+    """

It would be good to mention that if the compiled extension is not 
available
None is returned for the second element of the tuple.

+    if '.' in modulename:
+        package_name, name = modulename.rsplit('.', 1)
+        if '.' in package_name:
+            tail = package_name.rsplit('.', 1)[1]
+        else:
+            tail = package_name
+        package = __import__(package_name, globals(), locals(), tail)
+        path = package.__path__
+    else:
+        name = modulename
+        package = None
+        path = None

The case of loading a module with no dots is not actually tested.  Some 
options are:

1- remove support for this until we care enough to test it
2- leave it in and untested
3- do a test which loads some python top-level module - it would not 
need to have a C implementation to be a reasonable test
4- add a test module which is not inside bzrlib

I think I would prefer 3.

+        try:
+            fp = file(pathname_py, _PY_STUFF[1])
+        except IOError, e:
+            raise ImportError('cannot import python module %s (%s)' % (
+ 
modulename,
+ 
str(e)))
+        try:
+            m_py = imp.load_module(name+'_py', fp, pathname_py, 
_PY_STUFF)
+        finally:
+            fp.close()

I would wrap the exception string parameters onto the next line

       raise ImportError('cannot import python module %s (%s)'
                         % (modulename, str(e)))


For details, see: 
http://bundlebuggy.aaronbentley.com/request/%3C46B785AB.3090707%40ukr.net%3E



More information about the bazaar mailing list