AUTOMATING CODE QUALITY Kyle Knapp May 13th, 2018

WHAT DOES AUTOMATING CODE QUALITY ENTAIL? Reducing the number of manual code quality checks that does not require a human to perform

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.

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.

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.

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.

EXAMPLE: FOLLOWING SAFE PRACTICES some_object._internal_method() Unsafe usage: Accessing non-public methods from outside of class

WHY DOES AUTOMATING CODE QUALITY MATTER? Makes development processes safer and more efficient

CHALICE AUTOMATION

BENEFIT 1: ENFORCES QUALITY CHECKS

BENEFIT 2: IMPROVED CODE REVIEWS

BENEFIT 2: IMPROVED CODE REVIEWS PEP 8 violation: Line greater than 79 characters

BENEFIT 2: IMPROVED CODE REVIEWS

BENEFIT 3: THE MACHINE HAS YOUR BACK

STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment

STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment

STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment

pycodestyle pyflakes mccabe flake8

flake8 $ pip install flake8 $ flake8 mymodule.py $ flake8 mypackage/

pycodestyle pyflakes mccabe flake8

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:'

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:'

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:'

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

pycodestyle pyflakes mccabe flake8

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'

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'

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

pycodestyle pyflakes mccabe flake8

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

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.

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

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

STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment

pylint • Similar to flake8 • Checks coding standards • Warns for styling issues • Checks for potential bugs • Generally stricter and more opinionated than flake8

pylint $ pip install pylint $ pylint mymodule.py $ pylint mypackage/

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)

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)

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)

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)

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

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

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

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

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

STEPS TO AUTOMATION • Tools for improving code quality • flake8 • pylint • coverage • Automating for a local environment • Automating for a team/project environment

$ pip install coverage coverage $ pip install pytest-cov

~/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 ====================

~/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 ====================

.coveragerc ~/chalice$ py.test --cov chalice --cov-report term-missing tests/unit/ tests/functional/ [run] branch = True

~/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 ====================

~/chalice$ py.test --cov chalice --cov-report html tests/unit/ tests/functional/ ~/chalice$ open htmlcov/index.html Partial branch reason

CODE QUALITY TOOLS ECOSYSTEM Bugs Style Documentation Usability flake8 pylint coverage mypy pydocstyle doc8 There are many more!

STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment

. ── .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

# 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

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…

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

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

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

STEPS TO AUTOMATION • Tools for improving code quality • Automating for a local environment • Automating for a team/project environment

. ── .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

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

No content

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

No content

No content

No content

No content

No content

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

BEST PRACTICE 1: ALWAYS BE IMPROVING QUALITY CHECKS

BEST PRACTICE 1: ALWAYS BE IMPROVING QUALITY CHECKS

BEST PRACTICE 2: AVOID COMPROMISING EXISTING QUALITY CHECKS

BEST PRACTICE 3: BE AWARE AUTOMATED QUALITY CHECKS DOES NOT GUARANTEE CODE QUALITY

BEST PRACTICE 3: BE AWARE AUTOMATED QUALITY CHECKS DOES NOT GUARANTEE CODE QUALITY

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!

THANKS! • Python Code Quality Authority: https://github.com/PyCQA • Chalice repository: https://github.com/aws/chalice Kyle Knapp (@thekyleknapp) https://github.com/kyleknap