Motivation

Developers need to run several tasks and setting these up could be a hassle. These tasks include but are not limited to setting up the environment, running style checks, refactoring code based on best practices and running tests.

Tools like pylint, prospector etc. do a great job of giving users flexibility, but they are mostly run with a certain set of default values. They also do not give options like allowing to run on only currently touched files and similar heursitics. Documentation is needed for default values, environment required and so on for every tool per project. But we rarely read documentations, so an interactive script with default values and options to run these tools in a limited scope might be useful.

python/run.py allows us to do most of the things we need for development cycle and also guide us through the process using "--help" and autocomplete options. It should also be easy for us to maintain such a script and to read it to see what goes on under the hood. Luckily for us python argh package uses parseargs and argscomplete to expose python functions to shell with great help and autocompletion.

Current Tools

pylint is configured for variable names as both snake_case and camelCase
prospector allows some other tests like pep8 conventions and code complexity; we generate suggestions using this but do not enforce them in CI
yapf patches code automatically to follow pep8 and is not a part of CI; it is configured with defaults in the script
envsetup is a python function to allow setting environment for this project; see demo below
nose tests are currently used in run-tests.sh after setting some environment variables and specifying defaults

Future scope

Write pylint plugins that check for public APIs and allow them to be camelCase while keeping everything else snake_case. Public API can be defined as those considered public by sphinx by recursively using __all__
Move tests to run.py to allow maintainability, readability and add functionality.
Run tests, style checks etc. on "currently touched files", "currently touched submodules" and so on to speed up development and run CI in stages
Add mypy and pyannotate tools

Notes

We can specify commands and default arguments in run.py in a very pythonic way and this gets auto exposed while running the shell script without any more work by argh.
we no longer need to read complicated bash files, if bash gets too complicated, it's easier to write that in python and understand that
argh also generates help files from python docstrings
argcomplete generates autocomplete -- we can bootstrap that configuration through envsetup option

mypy is a static type checker for Python. If you sprinkle your code with type annotations, mypy can type check your code and find common bugs. [mypy docs] For example:

def f(x):
# type: (int) -> int
return x+1

if __name__ == '__main__':
print(f(41))
print(f("41"))

## $ mypy tmp.py
## tmp.py:7: error: Argument 1 to "f" has incompatible type "str"; expected "int"

pyannotate can help us generate these type annotations using testing. For example, assuming there's no type annotation currently in tmp.py, we can run the tests for it in following test_tmp.py and use the file generated with pyannotate to make the following patch.

from pyannotate_runtime import collect_types
from tmp import f

def test_f():
assert(f(41) == 42)

if __name__ == '__main__':
collect_types.init_types_collection()
with collect_types.collect():
  test_f()
collect_types.dump_stats('type_info.json')

## $ pyannotate -w --type-info type_info.json tmp.py

--- tmp.py      (original)
+++ tmp.py      (refactored)
@@ -1,4 +1,5 @@
def f(x):
+    # type: (int) -> int
   return x+1

if __name__ == '__main__':

Demo of python/run.py


$ python/run.py -h
usage: run.py [-h] {pylint,prospector,yapf,envsetup} ...

positional arguments: {pylint,prospector,yapf,envsetup} pylint ...(truncated for brevity)... prospector Wraps prospector and provides defaults. Run prospector --help for more details. Trailing arguments are a list of files, packages or modules. if nothing is specified, default value is ./python/sparkdl yapf Wraps yapf and provides some defaults. Run yapf --help for more details. envsetup ...(truncated for brevity)...

optional arguments: -h, --help show this help message and exit

$ python/run.py envsetup -h usage: run.py envsetup [-h] [-d] [-i] [-m] [-c] [-v]

Prints out shell commands that can be used in terminal to setup the environment.

This tool inspects the current environment, and/or adds default values, and/or interactively
asks user for values and prints all or the missing variables that need to be set. It can also
provide completion for this script. You can source the setup as follows:

```
python/run.py envsetup -d -c > ./python/.setup.sh && source ./python/.setup.sh
```

:param default: if default values should be set in this script or not
:param interactive: if user should be prompted for values or not
:param missing_only: if only missing variable should be printed
:param completion: if auto complete code should be printed
:param verbose: if user should be guided or not

optional arguments: -h, --help show this help message and exit -d, --default False -i, --interactive False -m, --missing-only False -c, --completion False -v, --verbose False

$ python/run.py pylint -h usage: run.py pylint [-h] [--rcfile RCFILE] [--reports REPORTS] [args [args ...]]

Wraps `pylint` and provides defaults. Run `prospector --help` for more details. Trailing
arguments are a list of files, packages or modules. if nothing is specified, default value is
./python/sparkdl

positional arguments: args list of files,packages or modules. if nothing is specified, default value is sparkdl (default: -)

optional arguments: -h, --help show this help message and exit --rcfile RCFILE './python/.pylint/accepted.rc' --reports REPORTS 'y'

codecov-io commented 6 years ago

Codecov Report

Merging #124 into master will not change coverage. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master     #124   +/-   ##
=======================================
  Coverage   85.37%   85.37%           
=======================================
  Files          33       33           
  Lines        1921     1921           
  Branches       44       44           
=======================================
  Hits         1640     1640           
  Misses        281      281

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update b09974b...8cc1679. Read the comment docs.

mengxr commented 6 years ago

@MrBago Could you take another pass?