Discussion:
[Swig-devel] PEP8 failures in python_moduleimport?
Vadim Zeitlin
2016-12-09 17:15:50 UTC
Permalink
Hi,

Travis CI seems not to mind somehow, but locally I'm getting test failures
since the recent 2a42031b085d8f4568aa90dede879ea008e1611b commit allowing
to define custom Python module importing code:

% make -C Examples/test-suite/python -s python_moduleimport.cpptest
checking python testcase python_moduleimport (with run test)
python_moduleimport.py:8:1: E265 block comment should start with '# '
python_moduleimport.py:10:1: E265 block comment should start with '# '
Makefile:107: recipe for target 'python_moduleimport.cpptest' failed
make: *** [python_moduleimport.cpptest] Error 1

Looking at the generated code, it does indeed contain "#print" lines which
result in this error.

The following trivial change:
---------------------------------- >8 --------------------------------------
diff --git a/Examples/test-suite/python_moduleimport.i b/Examples/test-suite/python_moduleimport.i
index 48b794c..f62547d 100644
--- a/Examples/test-suite/python_moduleimport.i
+++ b/Examples/test-suite/python_moduleimport.i
@@ -1,9 +1,9 @@
#if !defined(SWIGPYTHON_BUILTIN)
%define MODULEIMPORT
"
-#print 'Loading low-level module $module'
+# print 'Loading low-level module $module'
import $module
-#print 'Module has loaded'
+# print 'Module has loaded'
extra_import_variable = 'custom import of $module'
"
%enddef
@@ -11,10 +11,10 @@ extra_import_variable = 'custom import of $module'
#else
%define MODULEIMPORT
"
-#print 'Loading low-level module $module'
+# print 'Loading low-level module $module'
extra_import_variable = 'custom import of $module'
from $module import *
-#print 'Module has loaded'
+# print 'Module has loaded'
"
%enddef
#endif
---------------------------------- >8 --------------------------------------
fixes the test for me, but I wonder how does this work for Travis and,
presumably, for William. Am I missing something here? Would disabling E265
be a better fix?

Thanks,
VZ
William S Fulton
2016-12-11 22:37:30 UTC
Permalink
Post by Vadim Zeitlin
Hi,
Travis CI seems not to mind somehow, but locally I'm getting test failures
since the recent 2a42031b085d8f4568aa90dede879ea008e1611b commit allowing
% make -C Examples/test-suite/python -s python_moduleimport.cpptest
checking python testcase python_moduleimport (with run test)
python_moduleimport.py:8:1: E265 block comment should start with '# '
python_moduleimport.py:10:1: E265 block comment should start with '# '
Makefile:107: recipe for target 'python_moduleimport.cpptest' failed
make: *** [python_moduleimport.cpptest] Error 1
Looking at the generated code, it does indeed contain "#print" lines which
result in this error.
---------------------------------- >8 ------------------------------
--------
diff --git a/Examples/test-suite/python_moduleimport.i
b/Examples/test-suite/python_moduleimport.i
index 48b794c..f62547d 100644
--- a/Examples/test-suite/python_moduleimport.i
+++ b/Examples/test-suite/python_moduleimport.i
@@ -1,9 +1,9 @@
#if !defined(SWIGPYTHON_BUILTIN)
%define MODULEIMPORT
"
-#print 'Loading low-level module $module'
+# print 'Loading low-level module $module'
import $module
-#print 'Module has loaded'
+# print 'Module has loaded'
extra_import_variable = 'custom import of $module'
"
%enddef
@@ -11,10 +11,10 @@ extra_import_variable = 'custom import of $module'
#else
%define MODULEIMPORT
"
-#print 'Loading low-level module $module'
+# print 'Loading low-level module $module'
extra_import_variable = 'custom import of $module'
from $module import *
-#print 'Module has loaded'
+# print 'Module has loaded'
"
%enddef
#endif
---------------------------------- >8 ------------------------------
--------
fixes the test for me, but I wonder how does this work for Travis and,
presumably, for William. Am I missing something here? Would disabling E265
be a better fix?
There are two points of interest. The first is pep8 was only detected in
configure when using python3 and not python2.
The second is the version of pep8 on Travis was 1.4.0 which didn't seem to
detect E265. I've made a change
to use pep8 1.7.0 on Travis which did detect E265. I've also applied your
patch above. Many thanks.

William
Vadim Zeitlin
2016-12-11 23:27:55 UTC
Permalink
On Sun, 11 Dec 2016 22:37:30 +0000 William S Fulton <***@fultondesigns.co.uk> wrote:

WSF> There are two points of interest. The first is pep8 was only detected in
WSF> configure when using python3 and not python2.

Ah, I didn't think to check it, thanks for finding and fixing this!

WSF> The second is the version of pep8 on Travis was 1.4.0 which didn't
WSF> seem to detect E265. I've made a change to use pep8 1.7.0 on Travis
WSF> which did detect E265. I've also applied your patch above. Many
WSF> thanks.

Thanks to you for applying this! And I'm glad that at least this time it
was reproducible and had a reasonable explanation.

Unfortunately I also still struggle with another problem, for which I
don't have any explanation (reasonable or not), but which has a simple
workaround that I submitted as https://github.com/swig/swig/pull/688
If you could please also apply/merge this one, it would be really great
because I could then run Python test suite without having to apply any
patches to my tree first.

Thanks again,
VZ

Loading...