Kyle Knapp - Automating Code Quality

Writing quality Python code can be both tough and tedious. On top of the general design, there are many code quality aspects that you need to watch out for when writing and reviewing code such as adherence to PEP8, docstring quality, test quality, etc. Furthermore, everyone is human. If you are catching these code quality issues by hand, there is a good chance that at some point you will miss an easy opportunity to improve code quality. If the quality check can be done by a machine, then why would you even try to catch the code quality issue by hand? In the end, the machine will be able to perform the quality check with much more speed, accuracy, and consistency than a person.

This talk will dive into how existing open source projects offload and automate many of these code quality checks resulting in:

- A higher quality and a more consistent codebase
- Maintainers being able to focus more on the higher level design and interfaces
of a project.
- An improved contribution process and higher quality pull requests from
external contributors

By diving into how these open source projects automate code quality checks, you will learn about:

- The available tooling related to checking code quality such as `flake8`,
`pylint`, `coverage`, etc.
- How to automate code quality checks for both a development and team
setting.
- First-hand accounts of the benefits and lessons learned from automating
code quality checks in real-life open source projects.

https://us.pycon.org/2018/schedule/presentation/159/

Transcript

1. AUTOMATING CODE QUALITY Kyle Knapp May 13th, 2018
2. WHAT DOES AUTOMATING CODE QUALITY ENTAIL? Reducing the number of manual code quality checks that does not require a human to perform
3. def addNumbers(val1, val2): """ Print the sum of two numbers """ total=val1+val2 print(total) PEP 8 violation: Functions should be lowercase and separated by underscores PEP 8 violation: Surround assignment operator by whitespace on both sides EXAMPLE: FOLLOWING STANDARDS PEP 257 violation: One-line docstrings should fit on one line.
4. EXAMPLE: FOLLOWING SAFE PRACTICES def some_function(key, value, mutable_obj={}): mutable_obj[key] = value return mutable_obj >>> some_function('foo', 'bar') {'foo': 'bar'} >>> some_function('biz', 'baz') {'foo': 'bar', 'biz': 'baz'} Unsafe usage: Default mutable value for argument.
5. EXAMPLE: FOLLOWING SAFE PRACTICES def some_function(key, value, mutable_obj={}): mutable_obj[key] = value return mutable_obj >>> some_function('foo', 'bar') {'foo': 'bar'} >>> some_function('biz', 'baz') {'foo': 'bar', 'biz': 'baz'} Unsafe usage: Default mutable value for argument.
6. EXAMPLE: FOLLOWING SAFE PRACTICES def some_function(key, value, mutable_obj={}): mutable_obj[key] = value return mutable_obj >>> some_function('foo', 'bar') {'foo': 'bar'} >>> some_function('biz', 'baz') {'foo': 'bar', 'biz': 'baz'} Unsafe usage: Default mutable value for argument.
7. EXAMPLE: FOLLOWING SAFE PRACTICES some_object._internal_method() Unsafe usage: Accessing non-public methods from outside of class
8. WHY DOES AUTOMATING CODE QUALITY MATTER? Makes development processes safer and more efficient
9. CHALICE AUTOMATION
10. BENEFIT 1: ENFORCES QUALITY CHECKS
11. BENEFIT 2: IMPROVED CODE REVIEWS
12. BENEFIT 2: IMPROVED CODE REVIEWS PEP 8 violation: Line greater than 79 characters
13. BENEFIT 2: IMPROVED CODE REVIEWS
14. BENEFIT 3: THE MACHINE HAS YOUR BACK
15. STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment
16. STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment
17. STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment
18. pycodestyle pyflakes mccabe flake8
19. flake8 $ pip install flake8 $ flake8 mymodule.py $ flake8 mypackage/
20. pycodestyle pyflakes mccabe flake8
21. example1.py def print_evens(values): for val in values: is_even=((val % 2) == 0) if is_even == True: print('Found even value %s' % val) 1. 2. 3. 4. 5. $ flake8 example1.py example1.py:2:12: E272 multiple spaces before keyword example1.py:3:16: E225 missing whitespace around operator example1.py:4:20: E712 comparison to True should be 'if cond is True:' or 'if cond:'
22. example1.py def print_evens(values): for val in values: is_even=((val % 2) == 0) if is_even == True: print('Found even value %s' % val) 1. 2. 3. 4. 5. $ flake8 example1.py example1.py:2:12: E272 multiple spaces before keyword example1.py:3:16: E225 missing whitespace around operator example1.py:4:20: E712 comparison to True should be 'if cond is True:' or 'if cond:'
23. example1.py def print_evens(values): for val in values: is_even=((val % 2) == 0) if is_even == True: print('Found even value %s' % val) 1. 2. 3. 4. 5. $ flake8 example1.py example1.py:2:12: E272 multiple spaces before keyword example1.py:3:16: E225 missing whitespace around operator example1.py:4:20: E712 comparison to True should be 'if cond is True:' or 'if cond:'
24. example1.py def print_evens(values): for val in values: is_even = ((val % 2) == 0) if is_even: print('Found even value %s' % val) 1. 2. 3. 4. 5. $ flake8 example1.py
25. pycodestyle pyflakes mccabe flake8
26. example2.py import sys def print_is_divisible(dividend, divisor): is_divisible = ((dividend % divisor) == 0) if is_divisble: print('%s is divisible by %s' % (dividend, divisor)) else: print('%s is not divisible by %s' % (dividend, divisor)) 1. 2. 3. 4. 5. 6. 7. 8. 9. $ flake8 example2.py example2.py:1:1: F401 'sys' imported but unused example2.py:5:5: F841 local variable 'is_divisible' is assigned to but never used example2.py:6:8: F821 undefined name 'is_divisble'
27. example2.py import sys def print_is_divisible(dividend, divisor): is_divisible = ((dividend % divisor) == 0) if is_divisble: print('%s is divisible by %s' % (dividend, divisor)) else: print('%s is not divisible by %s' % (dividend, divisor)) 1. 2. 3. 4. 5. 6. 7. 8. 9. $ flake8 example2.py example2.py:1:1: F401 'sys' imported but unused example2.py:5:5: F841 local variable 'is_divisible' is assigned to but never used example2.py:6:8: F821 undefined name 'is_divisble'
28. example2.py def print_is_divisible(dividend, divisor): is_divisible = ((dividend % divisor) == 0) if is_divisible: print('%s is divisible by %s' % (dividend, divisor)) else: print('%s is not divisible by %s' % (dividend, divisor)) 1. 2. 3. 4. 5. 6. $ flake8 example2.py
29. pycodestyle pyflakes mccabe flake8
30. example3.py def identity(value): return value def contains(container, value): if value in container: return True else: return False def contains_all(container, values): for value in values: if not contains(container, value): return False return True 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. McCabe complexity: 1
31. example3.py McCabe complexity: 1 McCabe complexity: 2 def identity(value): return value def contains(container, value): if value in container: return True else: return False def contains_all(container, values): for value in values: if not contains(container, value): return False return True 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16.
32. example3.py McCabe complexity: 1 McCabe complexity: 2 McCabe complexity: 3 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. def identity(value): return value def contains(container, value): if value in container: return True else: return False def contains_all(container, values): for value in values: if not contains(container, value): return False return True
33. example3.py def identity(value): return value def contains(container, value): if value in container: return True else: return False def contains_all(container, values): for value in values: if not contains(container, value): return False return True McCabe complexity: 1 McCabe complexity: 2 McCabe complexity: 3 $ flake8 --max-complexity 2 example3.py example3.py:12:1: C901 'contains_all' is too complex (3) 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. 11. 12. 13. 14. 15. 16. As a rule of thumb, use the value of 10 for maximum complexity
34. STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment
35. pylint • Similar to flake8 • Checks coding standards • Warns for styling issues • Checks for potential bugs • Generally stricter and more opinionated than flake8
36. pylint $ pip install pylint $ pylint mymodule.py $ pylint mypackage/
37. example4.py 1. 2. 3. 4. 5. def loadInto(filename, dest={}): with open(filename) as f: loaded_json=json.loads(f.read()) dest.update(loaded_json) return dest $ pylint example4.py ************* Module example4 C: 3, 0: Exactly one space required around assignment loaded_json=json.loads(f.read()) ^ (bad-whitespace) C: 1, 0: Missing module docstring (missing-docstring) W: 1, 0: Dangerous default value {} as argument (dangerous-default-value) C: 1, 0: Invalid function name "loadInto" (hint: (([a-z][a-z0-9_]{2,50})|(_[a-z0-9_]*))$) (invalid-name) C: 1, 0: Missing function docstring (missing-docstring) C: 2,27: Invalid variable name "f" (hint: (([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$) (invalid-name) E: 3,20: Undefined variable 'json' (undefined-variable)
38. example4.py 1. 2. 3. 4. 5. def loadInto(filename, dest={}): with open(filename) as f: loaded_json=json.loads(f.read()) dest.update(loaded_json) return dest $ pylint example4.py ************* Module example4 C: 3, 0: Exactly one space required around assignment loaded_json=json.loads(f.read()) ^ (bad-whitespace) C: 1, 0: Missing module docstring (missing-docstring) W: 1, 0: Dangerous default value {} as argument (dangerous-default-value) C: 1, 0: Invalid function name "loadInto" (hint: (([a-z][a-z0-9_]{2,50})|(_[a-z0-9_]*))$) (invalid-name) C: 1, 0: Missing function docstring (missing-docstring) C: 2,27: Invalid variable name "f" (hint: (([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$) (invalid-name) E: 3,20: Undefined variable 'json' (undefined-variable)
39. example4.py 1. 2. 3. 4. 5. def loadInto(filename, dest={}): with open(filename) as f: loaded_json=json.loads(f.read()) dest.update(loaded_json) return dest $ pylint example4.py ************* Module example4 C: 3, 0: Exactly one space required around assignment loaded_json=json.loads(f.read()) ^ (bad-whitespace) C: 1, 0: Missing module docstring (missing-docstring) W: 1, 0: Dangerous default value {} as argument (dangerous-default-value) C: 1, 0: Invalid function name "loadInto" (hint: (([a-z][a-z0-9_]{2,50})|(_[a-z0-9_]*))$) (invalid-name) C: 1, 0: Missing function docstring (missing-docstring) C: 2,27: Invalid variable name "f" (hint: (([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$) (invalid-name) E: 3,20: Undefined variable 'json' (undefined-variable)
40. example4.py 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. import json def load_into(filename, dest=None): if dest is None: dest = {} with open(filename) as f: loaded_json = json.loads(f.read()) dest.update(loaded_json) return dest $ pylint example4.py ************* Module example4 C: 1, 0: Missing module docstring (missing-docstring) C: 1, 0: Missing function docstring (missing-docstring) C: 2,27: Invalid variable name "f" (hint: (([a-z][a-z0-9_]{2,30})|(_[a-z0-9_]*))$) (invalid-name)
41. example4.py 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. import json def load_into(filename, dest=None): if dest is None: dest = {} with open(filename) as f: loaded_json = json.loads(f.read()) dest.update(loaded_json) return dest $ pylint --generate-rcfile > .pylintrc
42. example4.py ... [MESSAGES CONTROL] disable=missing-docstring ... 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. import json def load_into(filename, dest=None): if dest is None: dest = {} with open(filename) as f: loaded_json = json.loads(f.read()) dest.update(loaded_json) return dest .pylintrc
43. example4.py ... [BASIC] good-names=i,j,k,ex,Run,_,f ... .pylintrc 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. import json def load_into(filename, dest=None): if dest is None: dest = {} with open(filename) as f: loaded_json = json.loads(f.read()) dest.update(loaded_json) return dest
44. example4.py 1. 2. 3. 4. 5. 6. 7. 8. 9. 10. import json def load_into(filename, dest=None): if dest is None: dest = {} with open(filename) as f: loaded_json = json.loads(f.read()) dest.update(loaded_json) return dest $ pylint --rcfile .pylintrc example4.py
45. pylint flake8 • Faster • Checks: • Stricter whitespace • McCabe complexity • Slower • Checks: • Names • Dangerous patterns • Maximums • DRY • Stricter error detection • Plugins • Configuration • Checks: • PEP 8 • Unused imports • Idiomatic statements • Variable referencing
46. STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment
47. $ pip install coverage coverage $ pip install pytest-cov
48. ~/chalice$ py.test tests/unit/ tests/functional/ ============================= test session starts ============================== ...[OMITTED]... tests/unit/test_analyzer.py ......................................................s ...[OMITTED]... tests/functional/cli/test_factory.py ................... ==================== 683 passed, 2 skipped in 18.67 seconds ====================
49. ~/chalice$ py.test --cov chalice --cov-report term-missing tests/unit/ tests/functional/ ============================= test session starts ============================== ...[OMITTED]... tests/unit/test_analyzer.py ......................................................s ...[OMITTED]... tests/functional/cli/test_factory.py ................... ---------- coverage: platform darwin, python 2.7.10-final-0 ---------- Name Stmts Miss Cover Missing ---------------------------------------------------------- chalice/__init__.py 3 0 100% chalice/analyzer.py 337 13 96% 92, 138, 157, 181, 186, 278, 469,... chalice/app.py 507 6 99% 27-30, 38, 170, 359, 489 ...[Redacted]... chalice/utils.py 156 1 99% 163 ---------------------------------------------------------- TOTAL 3690 123 97% ==================== 683 passed, 2 skipped in 26.91 seconds ====================
50. .coveragerc ~/chalice$ py.test --cov chalice --cov-report term-missing tests/unit/ tests/functional/ [run] branch = True
51. ~/chalice$ py.test --cov chalice --cov-report term-missing tests/unit/ tests/functional/ ============================= test session starts ============================== ...[OMITTED]... tests/unit/test_analyzer.py ......................................................s ...[OMITTED]... tests/functional/cli/test_factory.py ................... ---------- coverage: platform darwin, python 2.7.10-final-0 ---------- Name Stmts Miss Branch BrPart Cover Missing ------------------------------------------------------------------------ chalice/__init__.py 3 0 0 0 100% chalice/analyzer.py 337 13 114 14 94% 92, 138,... chalice/app.py 507 6 136 10 98% 27-30,... ...[REDACTED]... chalice/utils.py 156 1 26 1 99% 163, 162->163 ------------------------------------------------------------------------ TOTAL 3690 123 948 63 96% ==================== 683 passed, 2 skipped in 23.72 seconds ====================
52. ~/chalice$ py.test --cov chalice --cov-report html tests/unit/ tests/functional/ ~/chalice$ open htmlcov/index.html Partial branch reason
53. CODE QUALITY TOOLS ECOSYSTEM Bugs Style Documentation Usability flake8 pylint coverage mypy pydocstyle doc8 There are many more!
54. STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment
55. . ── .coveragerc ── .gitignore ── .hypothesis ── .pylintrc ── .travis.yml ── CHANGELOG.rst ── CONTRIBUTING.rst ── LICENSE ── MANIFEST.in ── Makefile ── NOTICE ── README.rst ── chalice/ ── docs/ ── requirements-dev.txt ── requirements-docs.txt ── scripts/ ── setup.cfg ── setup.py
    ── tests/ ~/chalice$ tree -L 1 -a
56. # Dev requirements, used for various linting tools coverage==4.3.4 flake8==3.3.0 tox==2.2.1 wheel==0.26.0 doc8==0.7.0 -e git://github.com/PyCQA/pylint.git@7cb3ffddfd96f5e099ca697f6b1e30e727544627#egg=pylint astroid==1.6.1 pytest-cov==2.4.0 pydocstyle==2.0.0 # Test requirements ... mypy==0.501; python_version >= '3.6' $ pip install –r requirements-dev.txt chalice/requirement-dev.txt
57. chalice/Makefile (1/2) TESTS=tests/unit tests/functional check: ###### FLAKE8 ##### # No unused imports, no undefined vars, flake8 --ignore=E731,W503 --exclude chalice/__init__.py,chalice/compat.py --max-complexity 10 chalice/ flake8 --ignore=E731,W503,F401 --max-complexity 10 chalice/compat.py flake8 tests/unit/ tests/functional/ tests/integration # # Proper docstring conventions according to pep257 # # pydocstyle --add-ignore=D100,D101,D102,D103,D104,D105,D204,D301 chalice/ pylint: ###### PYLINT ###### pylint --rcfile .pylintrc chalice # Run our custom linter on test code. pylint --load-plugins tests.linter --disable=I,E,W,R,C,F --enable C9999,C9998 tests/ typecheck: mypy --py2 --ignore-missing-imports --follow-imports=skip -p chalice --disallow-untyped-defs Continued…
58. chalice/Makefile (1/2) TESTS=tests/unit tests/functional check: ###### FLAKE8 ##### # No unused imports, no undefined vars, flake8 --ignore=E731,W503 --exclude chalice/__init__.py,chalice/compat.py --max-complexity 10 chalice/ flake8 --ignore=E731,W503,F401 --max-complexity 10 chalice/compat.py flake8 tests/unit/ tests/functional/ tests/integration # # Proper docstring conventions according to pep257 # # pydocstyle --add-ignore=D100,D101,D102,D103,D104,D105,D204,D301 chalice/ pylint: ###### PYLINT ###### pylint --rcfile .pylintrc chalice # Run our custom linter on test code. pylint --load-plugins tests.linter --disable=I,E,W,R,C,F --enable C9999,C9998 tests/ typecheck: mypy --py2 --ignore-missing-imports --follow-imports=skip -p chalice --disallow-untyped-defs $ make check
59. chalice/Makefile (1/2) TESTS=tests/unit tests/functional check: ###### FLAKE8 ##### # No unused imports, no undefined vars, flake8 --ignore=E731,W503 --exclude chalice/__init__.py,chalice/compat.py --max-complexity 10 chalice/ flake8 --ignore=E731,W503,F401 --max-complexity 10 chalice/compat.py flake8 tests/unit/ tests/functional/ tests/integration # # Proper docstring conventions according to pep257 # # pydocstyle --add-ignore=D100,D101,D102,D103,D104,D105,D204,D301 chalice/ pylint: ###### PYLINT ###### pylint --rcfile .pylintrc chalice # Run our custom linter on test code. pylint --load-plugins tests.linter --disable=I,E,W,R,C,F --enable C9999,C9998 tests/ typecheck: mypy --py2 --ignore-missing-imports --follow-imports=skip -p chalice --disallow-untyped-defs $ make pylint
60. test: py.test -v $(TESTS) coverage: py.test --cov chalice --cov-report term-missing $(TESTS) htmlcov: py.test --cov chalice --cov-report html $(TESTS) rm -rf /tmp/htmlcov && mv htmlcov /tmp/ open /tmp/htmlcov/index.html doccheck: ##### DOC8 ###### # Correct rst formatting for documentation # ## doc8 docs/source --ignore-path docs/source/topics/multifile.rst $(MAKE) -C docs linkcheck $(MAKE) -C docs html prcheck-py2: check pylint coverage doccheck prcheck: prcheck-py2 typecheck chalice/Makefile (2/2) $ make prcheck
61. STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment
62. . ── .coveragerc ── .gitignore ── .hypothesis ── .pylintrc ── .travis.yml ── CHANGELOG.rst ── CONTRIBUTING.rst ── LICENSE ── MANIFEST.in ── Makefile ── NOTICE ── README.rst ── chalice/ ── docs/ ── requirements-dev.txt ── requirements-docs.txt ── scripts/ ── setup.cfg ── setup.py
    ── tests/ ~/chalice$ tree -L 1 -a
63. language: python sudo: false env: HYPOTHESIS_PROFILE=ci matrix: include: - python: "3.6" env: TEST_TYPE=prcheck - python: "2.7" env: TEST_TYPE=prcheck-py2 install: - pip install -r requirements-dev.txt -r requirements-docs.txt - pip install -e . script: - make $TEST_TYPE after_success: - if [[ $TEST_TYPE == 'prcheck-py2' ]]; then pip install codecov==2.0.5 && codecov; fi chalice/.travis.yml
65. language: python sudo: false env: HYPOTHESIS_PROFILE=ci matrix: include: - python: "3.6" env: TEST_TYPE=prcheck - python: "2.7" env: TEST_TYPE=prcheck-py2 install: - pip install -r requirements-dev.txt -r requirements-docs.txt - pip install -e . script: - make $TEST_TYPE after_success: - if [[ $TEST_TYPE == 'prcheck-py2' ]]; then pip install codecov==2.0.5 && codecov; fi chalice/.travis.yml
71. BENEFITS OF AUTOMATED QUALITY CHECK SETUP • The machine is faster and more accurate than a human • Adding new quality checks is easy • Code must pass quality checks to be merged • Improved code review cycles
72. BEST PRACTICE 1: ALWAYS BE IMPROVING QUALITY CHECKS
73. BEST PRACTICE 1: ALWAYS BE IMPROVING QUALITY CHECKS
74. BEST PRACTICE 2: AVOID COMPROMISING EXISTING QUALITY CHECKS
75. BEST PRACTICE 3: BE AWARE AUTOMATED QUALITY CHECKS DOES NOT GUARANTEE CODE QUALITY
76. BEST PRACTICE 3: BE AWARE AUTOMATED QUALITY CHECKS DOES NOT GUARANTEE CODE QUALITY
77. OVERALL RECAP Managing tools and quality checks • requirements-dev.txt • Makefile Automating for a project environment • CI systems • Coverage services • Best practices Code quality tooling • flake8 • pylint • coverage • Many more!
78. THANKS! • Python Code Quality Authority: https://github.com/PyCQA • Chalice repository: https://github.com/aws/chalice Kyle Knapp (@thekyleknapp) https://github.com/kyleknap