From 43e49616b86ad45d9b0a1e301ac7ea7be372616b Mon Sep 17 00:00:00 2001
From: Chris Jewell <c.jewell@lancaster.ac.uk>
Date: Mon, 26 Aug 2019 15:56:49 +0100
Subject: [PATCH] Pylinted a bit.  Now just over 50% compliant.

---
 gem/gemlang/__init__.py           |  2 +-
 gem/gemlang/ast/__init__.py       |  5 ---
 gem/gemlang/ast/ast_base.py       |  9 +++-
 gem/gemlang/ast/ast_expression.py | 69 ++++++++++++++++---------------
 gem/gemlang/ast/ast_statement.py  |  7 +---
 gem/gemlang/ast/parsetree2ast.py  | 34 +++++++++------
 gem/gemlang/ast/utils.py          | 16 +++----
 gem/gemlang/model_generator.py    |  3 ++
 gem/model/edward2_extn.py         |  1 +
 pylintrc                          |  3 +-
 10 files changed, 83 insertions(+), 66 deletions(-)

diff --git a/gem/gemlang/__init__.py b/gem/gemlang/__init__.py
index f77614c..d05ef37 100644
--- a/gem/gemlang/__init__.py
+++ b/gem/gemlang/__init__.py
@@ -17,7 +17,7 @@
 #  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-from gem.gemlang.ast import serialize
+from gem.gemlang.ast.utils import serialize
 from gem.gemlang.ast.parsetree2ast import ASTValidator
 from gem.gemlang.ast_walker import ASTWalker
 from gem.gemlang.parse_gemlang import GEMParser
diff --git a/gem/gemlang/ast/__init__.py b/gem/gemlang/ast/__init__.py
index 3e76608..c64d460 100644
--- a/gem/gemlang/ast/__init__.py
+++ b/gem/gemlang/ast/__init__.py
@@ -22,11 +22,6 @@ import os
 from lark import Lark
 
 from gem import gemlang
-from gem.gemlang.ast.ast_base import *
-from gem.gemlang.ast.ast_expression import *
-from gem.gemlang.ast.ast_statement import *
-from gem.gemlang.ast.utils import *
-
 
 def make_ast_parser():
     glpath = os.path.dirname(inspect.getfile(gemlang))
diff --git a/gem/gemlang/ast/ast_base.py b/gem/gemlang/ast/ast_base.py
index a747cfc..9862641 100644
--- a/gem/gemlang/ast/ast_base.py
+++ b/gem/gemlang/ast/ast_base.py
@@ -17,7 +17,7 @@
 #  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-# Base classes for GEM Abstract Syntax Tree
+"""Base classes for GEM Abstract Syntax Tree"""
 
 __all__ = ['GEMProgram', 'Statement', 'ASTNode', 'NullNode',
            'Block', 'KwRef']  # Augmented automatically with operator classes
@@ -38,22 +38,29 @@ class ASTNode:
 
     @property
     def children(self):
+        """:return a list of child nodes."""
         return self.__children__
 
     @children.setter
     def children(self, children):
+        """:return a list of child nodes."""
         self.__children__ = children
 
     @property
     def meta(self):
+        """:return node metadata."""
         return self.__meta__
 
     def add_child(self, child):
+        """Adds a child node to the AST
+
+        :param child a child node of type ASTNode (or derived)"""
         if not isinstance(child, ASTNode):
             raise AssertionError
         self.__children__.append(child)
 
     def has_children(self):
+        """:return True if the node has children, else False"""
         return len(self.children) > 0
 
     def __str__(self):
diff --git a/gem/gemlang/ast/ast_expression.py b/gem/gemlang/ast/ast_expression.py
index 409afb6..f430fdb 100644
--- a/gem/gemlang/ast/ast_expression.py
+++ b/gem/gemlang/ast/ast_expression.py
@@ -17,35 +17,35 @@
 #  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-# AST Expression classes and parser
+"""AST Expression classes"""
 
-import operator as op
+import operator
 
 import numpy as np
 
 from gem.gemlang.ast.ast_base import Statement, ASTNode, KwRef
-from gem.gemlang.symbol import *
+from gem.gemlang.symbol import Scope
 
 __all__ = ['Expr', 'UnaryExpr', 'BinaryExpr', 'Call', 'KwArg',
            'IdRef', 'Number', 'String', 'TransitionDef', 'ArgList']
 
-binary_ops = ['truediv', 'mul', 'matmul', 'add', 'sub', 'pow', 'lt', 'gt',
-              'le', 'ge', 'eq', 'ne']
-unary_ops = ['neg', 'not_']
-
-op_symbols = {'truediv': '/',
-              'mul': '*',
-              'matmul': '@',
-              'add': '+',
-              'sub': '-',
-              'pow': '^',
-              'lt': '<',
-              'gt': '>',
-              'le': '<=',
-              'ge': '>=',
-              'ne': '!=',
-              'neg': '-',
-              'not_': '!'}
+_BINARY_OPS = ['truediv', 'mul', 'matmul', 'add', 'sub', 'pow', 'lt', 'gt',
+               'le', 'ge', 'eq', 'ne']
+_UNARY_OPS = ['neg', 'not_']
+
+_OP_SYMBOLS = {'truediv': '/',
+               'mul': '*',
+               'matmul': '@',
+               'add': '+',
+               'sub': '-',
+               'pow': '^',
+               'lt': '<',
+               'gt': '>',
+               'le': '<=',
+               'ge': '>=',
+               'ne': '!=',
+               'neg': '-',
+               'not_': '!'}
 
 
 class Expr(Statement):
@@ -55,9 +55,6 @@ class Expr(Statement):
     not intended to be used itself.
     """
 
-    def __init__(self, *args, **kwargs):
-        super().__init__(*args, **kwargs)
-
     def __str__(self):
         return str(self.__class__).split('.')[-1][:-2]
 
@@ -74,13 +71,13 @@ class UnaryExpr(Expr):
         self.add_child(x)
 
 
-for opname in unary_ops:
+for opname in _UNARY_OPS:
+    # pylint: disable=unnecessary-lambda
     typename = opname.capitalize() + "Expr"
     globals()[typename] = type(typename, (UnaryExpr,),
-                               {'__init__': lambda self, *args,
-                                                   **kwargs: UnaryExpr.__init__(
-                                   self, *args, **kwargs),
-                                'op': op.__dict__[opname]})
+                               {'__init__': lambda self, *args, **kwargs: UnaryExpr.__init__(self, *args,
+                                                                                             **kwargs),
+                                'op': operator.__dict__[opname]})
     __all__.append(typename)
 
 
@@ -98,13 +95,13 @@ class BinaryExpr(Expr):
 
 
 # Dynamically generate operator classes
-for opname in binary_ops:
+for opname in _BINARY_OPS:
+    # pylint: disable=unnecessary-lambda
     typename = opname.capitalize() + 'Expr'
     globals()[typename] = type(typename, (BinaryExpr,),
-                               {'__init__': lambda self, *args,
-                                                   **kwargs: BinaryExpr.__init__(
-                                   self, *args, **kwargs),
-                                'op': op.__dict__[opname]})
+                               {'__init__': lambda self, *args, **kwargs: BinaryExpr.__init__(self, *args,
+                                                                                              **kwargs),
+                                'op': operator.__dict__[opname]})
     __all__.append(typename)
 
 
@@ -144,10 +141,14 @@ class KwArg(Expr):
 
     @property
     def kwarg_scope(self):
+        """:return the keyword scope"""
         return self.__kwarg_scope
 
     @kwarg_scope.setter
     def kwarg_scope(self, scope):
+        """Sets a keyword argument scope
+
+        :param scope: the scope to set"""
         assert isinstance(scope, Scope)
         self.__kwarg_scope = scope
 
@@ -240,12 +241,14 @@ class ArgList(ASTNode):
 
     @property
     def kwarg_scope(self):
+        """:return the keyword scope"""
         return self.__kwarg_scope
 
     @kwarg_scope.setter
     def kwarg_scope(self, scope):
         assert isinstance(scope, Scope)
         self.__kwarg_scope = scope
+        # pylint: disable=expression-not-assigned
         [setattr(arg, 'kwarg_scope', scope) for arg in self.children if
          isinstance(arg, KwArg)]
 
diff --git a/gem/gemlang/ast/ast_statement.py b/gem/gemlang/ast/ast_statement.py
index b4c407e..d26dd74 100644
--- a/gem/gemlang/ast/ast_statement.py
+++ b/gem/gemlang/ast/ast_statement.py
@@ -17,9 +17,9 @@
 #  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-# Declaration parser
+"""Statement-related ASTNode derived classes defined here."""
 
-from gem.gemlang.ast.ast_base import *
+from gem.gemlang.ast.ast_base import Statement, Block
 from gem.gemlang.ast.ast_expression import IdRef, Expr, TransitionDef, ArgList
 
 __all__ = ['Decl', 'EpiDecl', 'AssignExpr', 'AssignTransition',
@@ -29,9 +29,6 @@ __all__ = ['Decl', 'EpiDecl', 'AssignExpr', 'AssignTransition',
 class Decl(Statement):
     """Declaration base class."""
 
-    def __init__(self, **kwargs):
-        super().__init__(**kwargs)
-
     def __str__(self):
         return "{type}".format(type=self.__class__.__name__)
 
diff --git a/gem/gemlang/ast/parsetree2ast.py b/gem/gemlang/ast/parsetree2ast.py
index 05bc05f..6942ad5 100644
--- a/gem/gemlang/ast/parsetree2ast.py
+++ b/gem/gemlang/ast/parsetree2ast.py
@@ -17,18 +17,25 @@
 #  IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE.
 #
 
-# Parse AST
+"""Classes and functions for translating parser tree to AST"""
+
+# pylint: disable=wildcard-import,unused-wildcard-import
 
 from lark import Transformer, v_args
 from lark import Tree as lark_tree
 
-from gem.gemlang.ast import *
-from gem.gemlang.ast import ASTNode
+from gem.gemlang.ast import ast_parser
+from gem.gemlang.ast.utils import serialize
+from gem.gemlang.ast.ast_base import GEMProgram, Block
+from gem.gemlang.ast.ast_expression import *
+from gem.gemlang.ast.ast_statement import EpiDecl, AssignExpr, AssignTransition, StochasticAssignExpr
+
 
 
 class Meta:
-    # N.B. we *could* use the Lark Meta class, but this isn't part of the documented interface and
-    # may be subject to change.
+    """This replaces the Lark Meta class.  We *could* use the Lark Meta class, but this isn't part of the
+documented interface and may be subject to change."""
+    # pylint: disable=too-few-public-methods
     def __init__(self, line, column):
         """Holds meta-information about where in the GEM source an operation lies."""
         self.line = line
@@ -38,19 +45,20 @@ class Meta:
 @v_args(inline=True)
 class ASTValidator(Transformer):
     """Validates an AST using a grammar.  May be used as a base class for optimisers"""
+    # pylint: disable=no-self-use,missing-docstring,undefined-variable,invalid-name,too-many-public-methods
 
-    def transform(self, ast):
+    def transform(self, tree):
         """Runs a tree transformer.
 
-        :param ast an instance of a string (serialized AST), ASTNode, or a Lark Tree object.
+        :param tree an instance of a string (serialized AST), ASTNode, or a Lark Tree object.
         :return an ASTNode object."""
 
-        if isinstance(ast, str):
-            return super().transform(ast_parser.parse(ast))
-        elif isinstance(ast, ASTNode):
-            return super().transform(ast_parser.parse(serialize(ast)))
-        elif isinstance(ast, lark_tree):
-            return super().transform(ast)
+        if isinstance(tree, str):
+            return super().transform(ast_parser.parse(tree))
+        elif isinstance(tree, ASTNode):
+            return super().transform(ast_parser.parse(serialize(tree)))
+        elif isinstance(tree, lark_tree):
+            return super().transform(tree)
         else:
             raise RuntimeError(
                 "AST is not an instance of 'str', 'ASTNode', or 'lark.Tree'")
diff --git a/gem/gemlang/ast/utils.py b/gem/gemlang/ast/utils.py
index d65b863..7bc365b 100644
--- a/gem/gemlang/ast/utils.py
+++ b/gem/gemlang/ast/utils.py
@@ -18,7 +18,9 @@
 #  #
 #
 
-from gem.gemlang.ast import ASTNode
+"""Utilities for ASTNodes, such as serialization"""
+
+from gem.gemlang.ast.ast_base import ASTNode
 
 __all__ = ['serialize', 'format_pretty']
 
@@ -29,10 +31,10 @@ def serialize(node):
     :param node: an AST node.
     """
     assert isinstance(node, ASTNode)
-    s = "{node}".format(node=str(node))
+    string = "{node}".format(node=str(node))
     for child in node.children:
-        s += " " + serialize(child)
-    return "({})".format(s)
+        string += " " + serialize(child)
+    return "({})".format(string)
 
 
 def format_pretty(node, level=0):
@@ -44,7 +46,7 @@ def format_pretty(node, level=0):
     :return a string containing the pretty printed representation of the AST.
     """
     assert isinstance(node, ASTNode)
-    s = "    " * level + str(node)
+    string = "    " * level + str(node)
     for child in node.children:
-        s += "\n " + format_pretty(child, level + 1)
-    return s
+        string += "\n " + format_pretty(child, level + 1)
+    return string
diff --git a/gem/gemlang/model_generator.py b/gem/gemlang/model_generator.py
index b3000e5..2668a8a 100644
--- a/gem/gemlang/model_generator.py
+++ b/gem/gemlang/model_generator.py
@@ -18,6 +18,8 @@
 #  #
 #
 
+
+
 import gem.gemlang.tf_output as out
 from gem.gemlang.ast import NullNode
 from gem.gemlang.ast_walker import ASTWalker
@@ -35,6 +37,7 @@ class ModelGenerator(ASTWalker):
     :param random_vars an object with field `free` and `observed` with lists of random vars.
     :return an Outputter object
     """
+    # pylint: disable=invalid-name,missing-docstring
 
     def __init__(self, symtab):
         super().__init__()
diff --git a/gem/model/edward2_extn.py b/gem/model/edward2_extn.py
index 735eb35..e4de5e1 100644
--- a/gem/model/edward2_extn.py
+++ b/gem/model/edward2_extn.py
@@ -69,6 +69,7 @@ class EpidemicProcess:
     def process(self):
         """Alias of `distribution`."""
         return self.__process
+
     @property
     def value(self):
         if self.__value is None:
diff --git a/pylintrc b/pylintrc
index 0916eda..026d761 100644
--- a/pylintrc
+++ b/pylintrc
@@ -138,7 +138,8 @@ disable=print-statement,
         xreadlines-attribute,
         deprecated-sys-function,
         exception-escape,
-        comprehension-escape
+        comprehension-escape,
+        no-else-return
 
 # Enable the message, report, category or checker with the given id(s). You can
 # either give multiple identifier separated by comma (,) or put this option
-- 
GitLab