Add checks to maintain a consistent Python code style and prevent bugs (#515)

A flake8 configuration is included in setup.cfg and the checks are
executed automatically on Travis:

* Ensures a consistent PEP8 code style
* Does basic linting to prevent possible bugs

.travis.yml

@@ -38,11 +38,12 @@ matrix:
   # Documentation build:
   - os: linux
     language: docs
-    env: DOCS STYLE
+    env: DOCS STYLE LINT
-    install: pip install sphinx sphinx_rtd_theme
+    install: pip install sphinx sphinx_rtd_theme flake8 pep8-naming
     script:
     - make -C docs html SPHINX_OPTIONS=-W
     - tools/check-style.sh
+    - flake8
 cache:
   directories:
   - $HOME/.cache/pip

pybind11/__init__.py

@@ -1,4 +1,4 @@
-from ._version import version_info, __version__
+from ._version import version_info, __version__  # noqa: F401 imported but unused
 
 
 def get_include(*args, **kwargs):

setup.cfg

@@ -1,2 +1,11 @@
 [bdist_wheel]
 universal=1
+
+[flake8]
+show_source = True
+exclude = .git, __pycache__, build, dist, docs, tools, venv
+ignore =
+    # line too long
+    E501,
+    # required for pretty matrix formating: multiple spaces after `,` and `[`
+    E201, E241

tests/conftest.py

@@ -8,7 +8,6 @@ import pytest
 import textwrap
 import difflib
 import re
-import os
 import sys
 import contextlib
 
@@ -218,7 +217,7 @@ def _test_import_pybind11():
     """
     # noinspection PyBroadException
     try:
-        import pybind11_tests
+        import pybind11_tests  # noqa: F401 imported but unused
     except Exception as e:
         print("Failed to import pybind11_tests from pytest:")
         print("  {}: {}".format(type(e).__name__, e))

tests/test_alias_initialization.py

@@ -1,12 +1,11 @@
-import pytest
 import gc
 
-def test_alias_delay_initialization(capture, msg):
 
-    # A only initializes its trampoline class when we inherit from it; if we
+def test_alias_delay_initialization1(capture):
-    # just create and use an A instance directly, the trampoline initialization
+    """A only initializes its trampoline class when we inherit from it; if we just
-    # is bypassed and we only initialize an A() instead (for performance
+    create and use an A instance directly, the trampoline initialization is bypassed
-    # reasons)
+    and we only initialize an A() instead (for performance reasons).
+    """
     from pybind11_tests import A, call_f
 
     class B(A):
@@ -37,14 +36,14 @@ def test_alias_delay_initialization(capture, msg):
         PyA.~PyA()
     """
 
-def test_alias_delay_initialization(capture, msg):
-    from pybind11_tests import A2, call_f
 
-    # A2, unlike the above, is configured to always initialize the alias; while
+def test_alias_delay_initialization2(capture):
-    # the extra initialization and extra class layer has small virtual dispatch
+    """A2, unlike the above, is configured to always initialize the alias; while
-    # performance penalty, it also allows us to do more things with the
+    the extra initialization and extra class layer has small virtual dispatch
-    # trampoline class such as defining local variables and performing
+    performance penalty, it also allows us to do more things with the trampoline
-    # construction/destruction.
+    class such as defining local variables and performing construction/destruction.
+    """
+    from pybind11_tests import A2, call_f
 
     class B2(A2):
         def __init__(self):

tests/test_chrono.py

@@ -47,7 +47,7 @@ def test_chrono_duration_roundtrip():
     from pybind11_tests import test_chrono3
     import datetime
 
-    # Get the difference betwen two times (a timedelta)
+    # Get the difference between two times (a timedelta)
     date1 = datetime.datetime.today()
     date2 = datetime.datetime.today()
     diff = date2 - date1

tests/test_class_args.py

@@ -1,5 +1,4 @@
 
-import pytest
 
 def test_class_args():
     # There's basically nothing to test here; just make sure the code compiled and declared its definition

tests/test_copy_move_policies.py

@@ -13,5 +13,3 @@ def test_lacking_move_ctor():
     with pytest.raises(RuntimeError) as excinfo:
         lacking_move_ctor.get_one()
     assert "the object is neither movable nor copyable!" in str(excinfo.value)
-
-

tests/test_docstring_options.py

@@ -1,6 +1,6 @@
-from pybind11_tests import ConstructorStats
 
-def test_docstring_options(capture):
+
+def test_docstring_options():
     from pybind11_tests import (test_function1, test_function2, test_function3,
                                 test_function4, test_function5, test_function6,
                                 test_function7, DocstringTestFoo)

tests/test_enum.py

@@ -30,10 +30,6 @@ def test_unscoped_enum():
     assert not (UnscopedEnum.ETwo < UnscopedEnum.EOne)
     assert not (2 < UnscopedEnum.EOne)
 
-def test_scoped_enum():
-    from pybind11_tests import ScopedEnum, test_scoped_enum
-
-    assert test_scoped_enum(ScopedEnum.Three) == "ScopedEnum::Three"
 
 def test_scoped_enum():
     from pybind11_tests import ScopedEnum, test_scoped_enum
@@ -56,6 +52,7 @@ def test_scoped_enum():
     assert ScopedEnum.Two >= ScopedEnum.Two
     assert ScopedEnum.Three >= ScopedEnum.Two
 
+
 def test_implicit_conversion():
     from pybind11_tests import ClassWithUnscopedEnum
 
@@ -87,6 +84,7 @@ def test_implicit_conversion():
     # Hashing test
     assert str(x) == "{EMode.EFirstMode: 3, EMode.ESecondMode: 4}"
 
+
 def test_binary_operators():
     from pybind11_tests import Flags
 
@@ -108,4 +106,3 @@ def test_binary_operators():
     state2 = ~state
     assert state2 == -7
     assert int(state ^ state2) == -1
-

tests/test_eval_call.py

@@ -1,4 +1,4 @@
 # This file is called from 'test_eval.py'
 
 if 'call_test2' in locals():
-    call_test2(y)
+    call_test2(y)  # noqa: F821 undefined name

tests/test_exceptions.py

@@ -69,6 +69,6 @@ def test_custom(msg):
     with pytest.raises(MyException5) as excinfo:
         try:
             throws5()
-        except MyException5_1 as e:
+        except MyException5_1:
             raise RuntimeError("Exception error: caught child from parent")
     assert msg(excinfo.value) == "this is a helper-defined translated exception"

tests/test_issues.py

@@ -83,7 +83,6 @@ def test_no_id(msg):
     assert expect_float(12) == 12
 
 
-
 def test_str_issue(msg):
     """Issue #283: __str__ called on uninitialized instance when constructor arguments invalid"""
     from pybind11_tests.issues import StrIssue
@@ -167,15 +166,17 @@ def test_override_ref():
 def test_operators_notimplemented(capture):
     from pybind11_tests.issues import OpTest1, OpTest2
     with capture:
-        C1, C2 = OpTest1(), OpTest2()
+        c1, c2 = OpTest1(), OpTest2()
-        C1 + C1
+        c1 + c1
-        C2 + C2
+        c2 + c2
-        C2 + C1
+        c2 + c1
-        C1 + C2
+        c1 + c2
-    assert capture == """Add OpTest1 with OpTest1
+    assert capture == """
+        Add OpTest1 with OpTest1
         Add OpTest2 with OpTest2
         Add OpTest2 with OpTest1
-Add OpTest2 with OpTest1"""
+        Add OpTest2 with OpTest1
+    """
 
 
 def test_iterator_rvpolicy():
@@ -185,7 +186,7 @@ def test_iterator_rvpolicy():
 
     assert list(make_iterator_1()) == [1, 2, 3]
     assert list(make_iterator_2()) == [1, 2, 3]
-    assert(type(make_iterator_1()) != type(make_iterator_2()))
+    assert not isinstance(make_iterator_1(), type(make_iterator_2()))
 
 
 def test_dupe_assignment():

tests/test_keep_alive.py

@@ -95,4 +95,3 @@ def test_return_none(capture):
         del p
         gc.collect()
     assert capture == "Releasing parent."
-

tests/test_methods_and_attributes.py

@@ -113,7 +113,8 @@ def test_property_return_value_policies(access):
 
 def test_property_rvalue_policy():
     """When returning an rvalue, the return value policy is automatically changed from
-       `reference(_internal)` to `move`. The following would not work otherwise."""
+    `reference(_internal)` to `move`. The following would not work otherwise.
+    """
     from pybind11_tests import TestPropRVP
 
     instance = TestPropRVP()

tests/test_multiple_inheritance.py

@@ -1,7 +1,6 @@
-import pytest
 
 
-def test_multiple_inheritance_cpp(msg):
+def test_multiple_inheritance_cpp():
     from pybind11_tests import MIType
 
     mt = MIType(3, 4)
@@ -10,7 +9,7 @@ def test_multiple_inheritance_cpp(msg):
     assert mt.bar() == 4
 
 
-def test_multiple_inheritance_mix1(msg):
+def test_multiple_inheritance_mix1():
     from pybind11_tests import Base2
 
     class Base1:
@@ -31,7 +30,7 @@ def test_multiple_inheritance_mix1(msg):
     assert mt.bar() == 4
 
 
-def test_multiple_inheritance_mix2(msg):
+def test_multiple_inheritance_mix2():
     from pybind11_tests import Base1
 
     class Base2:
@@ -52,7 +51,7 @@ def test_multiple_inheritance_mix2(msg):
     assert mt.bar() == 4
 
 
-def test_multiple_inheritance_virtbase(msg):
+def test_multiple_inheritance_virtbase():
     from pybind11_tests import Base12a, bar_base2a, bar_base2a_sharedptr
 
     class MITypePy(Base12a):

tests/test_numpy_array.py

@@ -144,10 +144,10 @@ def test_bounds_check(arr):
              mutate_data, mutate_data_t, at_t, mutate_at_t)
     for func in funcs:
         with pytest.raises(IndexError) as excinfo:
-            index_at(arr, 2, 0)
+            func(arr, 2, 0)
         assert str(excinfo.value) == 'index 2 is out of bounds for axis 0 with size 2'
         with pytest.raises(IndexError) as excinfo:
-            index_at(arr, 0, 4)
+            func(arr, 0, 4)
         assert str(excinfo.value) == 'index 4 is out of bounds for axis 1 with size 3'
 
 
@@ -166,50 +166,49 @@ def test_make_c_f_array():
 def test_wrap():
     from pybind11_tests.array import wrap
 
-    def assert_references(A, B):
+    def assert_references(a, b):
-        assert A is not B
+        assert a is not b
-        assert A.__array_interface__['data'][0] == \
+        assert a.__array_interface__['data'][0] == b.__array_interface__['data'][0]
-               B.__array_interface__['data'][0]
+        assert a.shape == b.shape
-        assert A.shape == B.shape
+        assert a.strides == b.strides
-        assert A.strides == B.strides
+        assert a.flags.c_contiguous == b.flags.c_contiguous
-        assert A.flags.c_contiguous == B.flags.c_contiguous
+        assert a.flags.f_contiguous == b.flags.f_contiguous
-        assert A.flags.f_contiguous == B.flags.f_contiguous
+        assert a.flags.writeable == b.flags.writeable
-        assert A.flags.writeable == B.flags.writeable
+        assert a.flags.aligned == b.flags.aligned
-        assert A.flags.aligned == B.flags.aligned
+        assert a.flags.updateifcopy == b.flags.updateifcopy
-        assert A.flags.updateifcopy == B.flags.updateifcopy
+        assert np.all(a == b)
-        assert np.all(A == B)
+        assert not b.flags.owndata
-        assert not B.flags.owndata
+        assert b.base is a
-        assert B.base is A
+        if a.flags.writeable and a.ndim == 2:
-        if A.flags.writeable and A.ndim == 2:
+            a[0, 0] = 1234
-            A[0, 0] = 1234
+            assert b[0, 0] == 1234
-            assert B[0, 0] == 1234
 
-    A1 = np.array([1, 2], dtype=np.int16)
+    a1 = np.array([1, 2], dtype=np.int16)
-    assert A1.flags.owndata and A1.base is None
+    assert a1.flags.owndata and a1.base is None
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
-    A1 = np.array([[1, 2], [3, 4]], dtype=np.float32, order='F')
+    a1 = np.array([[1, 2], [3, 4]], dtype=np.float32, order='F')
-    assert A1.flags.owndata and A1.base is None
+    assert a1.flags.owndata and a1.base is None
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
-    A1 = np.array([[1, 2], [3, 4]], dtype=np.float32, order='C')
+    a1 = np.array([[1, 2], [3, 4]], dtype=np.float32, order='C')
-    A1.flags.writeable = False
+    a1.flags.writeable = False
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
-    A1 = np.random.random((4, 4, 4))
+    a1 = np.random.random((4, 4, 4))
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
-    A1 = A1.transpose()
+    a1 = a1.transpose()
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
-    A1 = A1.diagonal()
+    a1 = a1.diagonal()
-    A2 = wrap(A1)
+    a2 = wrap(a1)
-    assert_references(A1, A2)
+    assert_references(a1, a2)
 
 
 @pytest.requires_numpy

tests/test_python_types.py

@@ -314,6 +314,7 @@ def test_optional():
     assert test_nullopt(42) == 42
     assert test_nullopt(43) == 43
 
+
 @pytest.mark.skipif(not has_exp_optional, reason='no <experimental/optional>')
 def test_exp_optional():
     from pybind11_tests import double_or_zero_exp, half_or_none_exp, test_nullopt_exp

tests/test_sequences_and_iterators.py

@@ -2,7 +2,7 @@ import pytest
 
 
 def isclose(a, b, rel_tol=1e-05, abs_tol=0.0):
-    """Like to math.isclose() from Python 3.5"""
+    """Like math.isclose() from Python 3.5"""
     return abs(a - b) <= max(rel_tol * max(abs(a), abs(b)), abs_tol)
 
 

tests/test_smart_ptr.py

@@ -114,7 +114,8 @@ def test_smart_ptr(capture):
     assert cstats.copy_assignments == 30
     assert cstats.move_assignments == 0
 
-def test_unique_nodelete(capture):
+
+def test_unique_nodelete():
     from pybind11_tests import MyObject4
     o = MyObject4(23)
     assert o.value == 23

tests/test_stl_binders.py

@@ -50,6 +50,7 @@ def test_vector_bool():
         assert vv_c[i] == (i % 2 == 0)
     assert str(vv_c) == "VectorBool[1, 0, 1, 0, 1, 0, 1, 0, 1, 0]"
 
+
 def test_map_string_double():
     from pybind11_tests import MapStringDouble, UnorderedMapStringDouble
 
@@ -57,30 +58,17 @@ def test_map_string_double():
     m['a'] = 1
     m['b'] = 2.5
 
-    keys = []
+    assert list(m) == ['a', 'b']
-    for k in m: keys.append(k)
+    assert list(m.items()) == [('a', 1), ('b', 2.5)]
-    assert keys == ['a', 'b']
-
-    key_values = []
-    for k, v in m.items(): key_values.append( (k, v) )
-    assert key_values == [('a', 1), ('b', 2.5) ]
-
     assert str(m) == "MapStringDouble{a: 1, b: 2.5}"
 
-
     um = UnorderedMapStringDouble()
     um['ua'] = 1.1
     um['ub'] = 2.6
 
-    keys = []
+    assert sorted(list(um)) == ['ua', 'ub']
-    for k in um: keys.append(k)
+    assert sorted(list(um.items())) == [('ua', 1.1), ('ub', 2.6)]
-    assert sorted(keys) == ['ua', 'ub']
+    assert "UnorderedMapStringDouble" in str(um)
-
-    key_values = []
-    for k, v in um.items(): key_values.append( (k, v) )
-    assert sorted(key_values) == [('ua', 1.1), ('ub', 2.6) ]
-
-    str(um)
 
 
 def test_map_string_double_const():
@@ -97,57 +85,56 @@ def test_map_string_double_const():
 
     str(umc)
 
+
 def test_noncopyable_vector():
-    from pybind11_tests import ENC, get_vnc
+    from pybind11_tests import get_vnc
 
     vnc = get_vnc(5)
     for i in range(0, 5):
-        assert(vnc[i].value == i+1)
+        assert vnc[i].value == i + 1
+
+    for i, j in enumerate(vnc, start=1):
+        assert j.value == i
 
-    i = 1
-    for j in vnc:
-        assert(j.value == i)
-        i += 1
 
 def test_noncopyable_deque():
-    from pybind11_tests import ENC, get_dnc
+    from pybind11_tests import get_dnc
 
     dnc = get_dnc(5)
     for i in range(0, 5):
-        assert(dnc[i].value == i+1)
+        assert dnc[i].value == i + 1
 
     i = 1
     for j in dnc:
         assert(j.value == i)
         i += 1
 
+
 def test_noncopyable_map():
-    from pybind11_tests import ENC, get_mnc
+    from pybind11_tests import get_mnc
 
     mnc = get_mnc(5)
     for i in range(1, 6):
-        assert(mnc[i].value == 10*i)
+        assert mnc[i].value == 10 * i
 
-    i = 1
     vsum = 0
     for k, v in mnc.items():
-        assert(v.value == 10*k)
+        assert v.value == 10 * k
         vsum += v.value
 
-    assert(vsum == 150)
+    assert vsum == 150
+
 
 def test_noncopyable_unordered_map():
-    from pybind11_tests import ENC, get_umnc
+    from pybind11_tests import get_umnc
 
     mnc = get_umnc(5)
     for i in range(1, 6):
-        assert(mnc[i].value == 10*i)
+        assert mnc[i].value == 10 * i
 
-    i = 1
     vsum = 0
     for k, v in mnc.items():
-        assert(v.value == 10*k)
+        assert v.value == 10 * k
         vsum += v.value
 
-    assert(vsum == 150)
+    assert vsum == 150
-

tests/test_virtual_functions.py

@@ -76,20 +76,20 @@ def test_override(capture, msg):
 def test_inheriting_repeat():
     from pybind11_tests import A_Repeat, B_Repeat, C_Repeat, D_Repeat, A_Tpl, B_Tpl, C_Tpl, D_Tpl
 
-    class VI_AR(A_Repeat):
+    class AR(A_Repeat):
         def unlucky_number(self):
             return 99
 
-    class VI_AT(A_Tpl):
+    class AT(A_Tpl):
         def unlucky_number(self):
             return 999
 
-    obj = VI_AR()
+    obj = AR()
     assert obj.say_something(3) == "hihihi"
     assert obj.unlucky_number() == 99
     assert obj.say_everything() == "hi 99"
 
-    obj = VI_AT()
+    obj = AT()
     assert obj.say_something(3) == "hihihi"
     assert obj.unlucky_number() == 999
     assert obj.say_everything() == "hi 999"
@@ -106,46 +106,46 @@ def test_inheriting_repeat():
         assert obj.lucky_number() == 888.0
         assert obj.say_everything() == "B says hi 1 times 4444"
 
-    class VI_CR(C_Repeat):
+    class CR(C_Repeat):
         def lucky_number(self):
             return C_Repeat.lucky_number(self) + 1.25
 
-    obj = VI_CR()
+    obj = CR()
     assert obj.say_something(3) == "B says hi 3 times"
     assert obj.unlucky_number() == 4444
     assert obj.lucky_number() == 889.25
     assert obj.say_everything() == "B says hi 1 times 4444"
 
-    class VI_CT(C_Tpl):
+    class CT(C_Tpl):
         pass
 
-    obj = VI_CT()
+    obj = CT()
     assert obj.say_something(3) == "B says hi 3 times"
     assert obj.unlucky_number() == 4444
     assert obj.lucky_number() == 888.0
     assert obj.say_everything() == "B says hi 1 times 4444"
 
-    class VI_CCR(VI_CR):
+    class CCR(CR):
         def lucky_number(self):
-            return VI_CR.lucky_number(self) * 10
+            return CR.lucky_number(self) * 10
 
-    obj = VI_CCR()
+    obj = CCR()
     assert obj.say_something(3) == "B says hi 3 times"
     assert obj.unlucky_number() == 4444
     assert obj.lucky_number() == 8892.5
     assert obj.say_everything() == "B says hi 1 times 4444"
 
-    class VI_CCT(VI_CT):
+    class CCT(CT):
         def lucky_number(self):
-            return VI_CT.lucky_number(self) * 1000
+            return CT.lucky_number(self) * 1000
 
-    obj = VI_CCT()
+    obj = CCT()
     assert obj.say_something(3) == "B says hi 3 times"
     assert obj.unlucky_number() == 4444
     assert obj.lucky_number() == 888000.0
     assert obj.say_everything() == "B says hi 1 times 4444"
 
-    class VI_DR(D_Repeat):
+    class DR(D_Repeat):
         def unlucky_number(self):
             return 123
 
@@ -158,15 +158,15 @@ def test_inheriting_repeat():
         assert obj.lucky_number() == 888.0
         assert obj.say_everything() == "B says hi 1 times 4444"
 
-    obj = VI_DR()
+    obj = DR()
     assert obj.say_something(3) == "B says hi 3 times"
     assert obj.unlucky_number() == 123
     assert obj.lucky_number() == 42.0
     assert obj.say_everything() == "B says hi 1 times 123"
 
-    class VI_DT(D_Tpl):
+    class DT(D_Tpl):
         def say_something(self, times):
-            return "VI_DT says:" + (' quack' * times)
+            return "DT says:" + (' quack' * times)
 
         def unlucky_number(self):
             return 1234
@@ -174,32 +174,35 @@ def test_inheriting_repeat():
         def lucky_number(self):
             return -4.25
 
-    obj = VI_DT()
+    obj = DT()
-    assert obj.say_something(3) == "VI_DT says: quack quack quack"
+    assert obj.say_something(3) == "DT says: quack quack quack"
     assert obj.unlucky_number() == 1234
     assert obj.lucky_number() == -4.25
-    assert obj.say_everything() == "VI_DT says: quack 1234"
+    assert obj.say_everything() == "DT says: quack 1234"
 
-    class VI_DT2(VI_DT):
+    class DT2(DT):
         def say_something(self, times):
-            return "VI_DT2: " + ('QUACK' * times)
+            return "DT2: " + ('QUACK' * times)
 
         def unlucky_number(self):
             return -3
 
-    class VI_BT(B_Tpl):
+    class BT(B_Tpl):
         def say_something(self, times):
-            return "VI_BT" * times
+            return "BT" * times
+
         def unlucky_number(self):
             return -7
+
         def lucky_number(self):
             return -1.375
 
-    obj = VI_BT()
+    obj = BT()
-    assert obj.say_something(3) == "VI_BTVI_BTVI_BT"
+    assert obj.say_something(3) == "BTBTBT"
     assert obj.unlucky_number() == -7
     assert obj.lucky_number() == -1.375
-    assert obj.say_everything() == "VI_BT -7"
+    assert obj.say_everything() == "BT -7"
+
 
 @pytest.mark.skipif(not hasattr(pybind11_tests, 'NCVirt'),
                     reason="NCVirt test broken on ICPC")