[Lint] Apply lint (#126)

Should be merged after #120 
fixes #125 

1. add `auto-formatter` **lint.sh** (feel free to add on others)
2. flake8, black, and isort formatting is clean now
3. pylint and mypy is a lot of work to address - need everyone address
these step by step (also take them out from style.sh, and make
style_optional.sh CI test for them.)
4. relative path addressed (import module instead of class and function)
5. We should enable Lint/Enforce as mandatory
6. Move debuginfo stand alone as it caused circular import.

NOTE: I hope this one can be quickly merged, as there are PRs being
merged constantly which means endless merge conflict for this one.

Co-authored-by: Ubuntu <titaiwang@titaiwanglinuxcpudev.y3zdd0j2xrqelnmezcgpqgmnte.jx.internal.cloudapp.net>

Author: titaiwangms

README.md

@@ -155,4 +155,3 @@ optional check:
 ```
 
 NOTE: mypy and pylint needs to be manually address
-

onnxscript/__init__.py

@@ -4,8 +4,10 @@
 # --------------------------------------------------------------------------
 
 import sys
-from .main import script, export_onnx_lib, OnnxFunction
+
 from .backend.onnx_export import export2python as proto2python
+from .main import export_onnx_lib, script
+from .values import OnnxFunction
 
 if sys.version_info[0:2] >= (3, 8):
     import importlib.metadata as importlib_metadata

onnxscript/__main__.py

@@ -5,11 +5,13 @@
 
 import os
 from typing import Optional
-import onnx
-import onnx.helper as helper
+
 import click
-from onnxscript.converter import Converter
-from onnxscript.backend.onnx_export import export2python
+import onnx
+from onnx import helper
+
+from onnxscript import converter
+from onnxscript.backend import onnx_export
 
 
 @click.group()
@@ -18,18 +20,18 @@ def cli():
 
 
 def convert_file(script):
-    converter = Converter()
-    return converter.convert_file(script)
+    convert = converter.Converter()
+    return convert.convert_file(script)
 
 
 def to_single_model_proto(model, input_py_file: str, output_onnx_file: Optional[str] = None):
-    if (not output_onnx_file):
-        prefix, ext = os.path.splitext(input_py_file)
+    if not output_onnx_file:
+        prefix, _ = os.path.splitext(input_py_file)
         output_onnx_file = prefix + ".onnx"
 
     fnlist = convert_file(input_py_file)
 
-    if (not fnlist):
+    if not fnlist:
         print("No functions in input.")
         return
 
@@ -47,8 +49,9 @@ def to_single_model_proto(model, input_py_file: str, output_onnx_file: Optional[
     model = onnx.helper.make_model(
         graph,
         functions=[f.to_function_proto() for f in fnlist],
-        producer_name='p2o',
-        opset_imports=[onnx.helper.make_opsetid("", 15)])
+        producer_name="p2o",
+        opset_imports=[onnx.helper.make_opsetid("", 15)],
+    )
 
     # TODO: add options for user to specify whether to check generated model
     # model = onnx.shape_inference.infer_shapes(model)
@@ -71,34 +74,52 @@ def print_ir_function(f):
 
 
 @cli.command()
-@click.option('--fmt', type=click.Choice(['text', 'model', 'lib'], case_sensitive=False),
-              help="Translate input to a single ModelProto ('model'), "
-                   "into a LibProto ('lib'), "
-                   "or into text 'text').")
-@click.option('name', '--name', envvar='PATHS', multiple=True, type=click.Path(),
-              help="File or files to convert.")
-def translate(fmt="text", name=None):
+@click.option(
+    "--fmt",
+    type=click.Choice(["text", "model", "lib"], case_sensitive=False),
+    help="Translate input to a single ModelProto ('model'), "
+    "into a LibProto ('lib'), "
+    "or into text 'text').",
+)
+@click.option(
+    "name",
+    "--name",
+    envvar="PATHS",
+    multiple=True,
+    type=click.Path(),
+    help="File or files to convert.",
+)
+def translate(fmt="text", names=None):
     """Translate a file or many files into a ModelProto, a LibProto or text."""
     if fmt == "text":
-        for name in name:
+        for name in names:
             to_text(name)
     else:
-        for name in name:
+        for name in names:
             to_single_model_proto(fmt == "model", name)
 
 
 @cli.command()
-@click.option('name', '--name', envvar='PATHS', multiple=False, type=click.Path(),
-              help="filename to convert")
-@click.option("--op", is_flag=True, default=False,
-              help="converts a numerical operator into op.Add (False) or keep it (True)")
-@click.option("--rename", is_flag=True, default=False,
-              help="to use shorter variable name")
+@click.option(
+    "name",
+    "--name",
+    envvar="PATHS",
+    multiple=False,
+    type=click.Path(),
+    help="filename to convert",
+)
+@click.option(
+    "--op",
+    is_flag=True,
+    default=False,
+    help="converts a numerical operator into op.Add (False) or keep it (True)",
+)
+@click.option("--rename", is_flag=True, default=False, help="to use shorter variable name")
 def onnx2script(name, op=False, rename=False):
     """Exports an onnx graph to a script in following onnx-script syntax.
     The result is printed on the standard output.
     """
-    code = export2python(name, use_operators=op, rename=rename)
+    code = onnx_export.export2python(name, use_operators=op, rename=rename)
     print(code)
 
 

onnxscript/analysis.py

@@ -4,18 +4,22 @@
 # --------------------------------------------------------------------------
 
 import ast
-from .values import DebugInfo
+
+from onnxscript import debuginfo
 
 
 def get_loop_var(for_stmt, converter):
     if not isinstance(for_stmt.target, ast.Name):
-        raise ValueError(DebugInfo(for_stmt, converter).msg(
-            "For loop target must be a single variable."))
+        raise ValueError(
+            debuginfo.DebugInfo(for_stmt, converter).msg(
+                "For loop target must be a single variable."
+            )
+        )
     return for_stmt.target.id
 
 
 def used_vars(expr):
-    ''' Return set of all variables used in an expression.'''
+    """Return set of all variables used in an expression."""
     if isinstance(expr, ast.Name):
         return set([expr.id])
     if isinstance(expr, ast.Call):
@@ -31,22 +35,24 @@ def used_vars(expr):
 
 
 def local_defs(lhs):
-    '''Utility function to return set of assigned/defined
-    variables in the lhs of an assignment statement.'''
+    """Utility function to return set of assigned/defined
+    variables in the lhs of an assignment statement."""
+
     def get_id(e):
         assert isinstance(e, ast.Name), "Only simple assignments supported."
         return e.id
 
-    if (isinstance(lhs, ast.Tuple)):
-        return set([get_id(x) for x in lhs.elts])
-    return set([get_id(lhs)])
+    if isinstance(lhs, ast.Tuple):
+        return {get_id(x) for x in lhs.elts}
+    return {get_id(lhs)}
 
 
 def defs(stmt):
-    '''
+    """
     Return the set of all variables that may be defined (assigned to) in an
     execution of input stmt.
-    '''
+    """
+
     def block_defs(block):
         result = set()
         for s in block:
@@ -66,7 +72,7 @@ def block_defs(block):
     if isinstance(stmt, ast.Break):
         return set()
     try:
-        if stmt.value.func.id == 'print':
+        if stmt.value.func.id == "print":
             # Any call to print function are ignored.
             return set()
     except (TypeError, AttributeError):
@@ -75,11 +81,12 @@ def block_defs(block):
 
 
 def do_liveness_analysis(fun, converter):
-    '''
+    """
     Perform liveness analysis of the given function-ast. The results of the
     analysis are stored directly with each statement-ast `s` as attributes `s.live_in`
     and `s.live_out`.
-    '''
+    """
+
     def visit(stmt, live_out):
         stmt.live_out = live_out
         live = do_visit(stmt, live_out)
@@ -124,22 +131,25 @@ def visitBlock(block, live_out):
             # Break statements in the middle of the loop, however, will require
             # a generalization.
             return live_out
-        if isinstance(stmt, ast.Expr) and hasattr(stmt, 'value'):
+        if isinstance(stmt, ast.Expr) and hasattr(stmt, "value"):
             # docstring
-            if hasattr(stmt.value, 'value') and isinstance(stmt.value.value, str):
+            if hasattr(stmt.value, "value") and isinstance(stmt.value.value, str):
                 # python 3.8+
                 return live_out
-            if hasattr(stmt.value, 's') and isinstance(stmt.value.s, str):
+            if hasattr(stmt.value, "s") and isinstance(stmt.value.s, str):
                 # python 3.7
                 return live_out
         try:
-            if stmt.value.func.id == 'print':
+            if stmt.value.func.id == "print":
                 # Any call to print function are ignored.
                 return live_out
         except (TypeError, AttributeError):
             pass
-        raise ValueError(DebugInfo(stmt, converter).msg(
-            f"Unsupported statement type {type(stmt)!r}."))
+        raise ValueError(
+            debuginfo.DebugInfo(stmt, converter).msg(
+                f"Unsupported statement type {type(stmt)!r}."
+            )
+        )
 
     assert isinstance(fun, ast.FunctionDef)
     live = set()
@@ -148,7 +158,7 @@ def visitBlock(block, live_out):
 
 
 def exposed_uses(stmts, converter):
-    '''
+    """
     Return the set of variables that are used before being defined by given block.
     In essence, this identifies the "inputs" to a given code-block.
     For example, consider the following code-block:
@@ -163,7 +173,8 @@ def exposed_uses(stmts, converter):
     the block. Even though the value of y is used within the block, it is assigned
     a value before it is used. However, in contrast, the incoming value of x is used
     (in the first statement). Hence x is included in the exposed_uses.
-    '''
+    """
+
     def visitBlock(block, live_out):
         for stmt in reversed(block):
             live_out = visit(stmt, live_out)
@@ -180,10 +191,13 @@ def visit(stmt, live_out):
             live1 = visitBlock(stmt.body, live_out)
             live2 = visitBlock(stmt.orelse, live_out)
             return (live1 | live2) | used_vars(stmt.test)
-        if (isinstance(stmt, ast.Expr) and hasattr(stmt, 'value') and
-                isinstance(stmt.value, ast.Call)):
+        if (
+            isinstance(stmt, ast.Expr)
+            and hasattr(stmt, "value")
+            and isinstance(stmt.value, ast.Call)
+        ):
             f = stmt.value.func
-            if f.id == 'print':
+            if f.id == "print":
                 return live_out
         if isinstance(stmt, ast.For):
             # Analysis assumes loop may execute zero times. Results can be improved
@@ -201,7 +215,10 @@ def visit(stmt, live_out):
             return used_inside_loop | used_in_loop_header | live_out
         if isinstance(stmt, ast.Break):
             return live_out
-        raise ValueError(DebugInfo(stmt, converter).msg(
-            f"Unsupported statement type {type(stmt)!r}."))
+        raise ValueError(
+            debuginfo.DebugInfo(stmt, converter).msg(
+                f"Unsupported statement type {type(stmt)!r}."
+            )
+        )
 
     return visitBlock(stmts, set())