Add pylintrc, flake8, pre-commit for Python scripts. Fix outstanding …

…warnings / errors. (#35)

* Add .flake8 and pylintrc, and .pre-commit-config.yaml for enforcement

* Fix code quality warnings (or ignore them)

Mostly silly stuff, but a few important fixes
* replace file() with open() (python 3+)
* Catch a few renames that were missed in the rebranding from tilt_brush
  to open_brush
* python3 style super() and class definitions
* Tweak the keyring import slightly
* There's a possible bug in bvh.py, when iterating and returning leafs.
  See the TODO
* Replace some map()s with list and set comprehensions

* Use utf-8 encoding for handling byte->str in git subprocess

* Add rewritten Support/ThirdParty/GeneratedThirdPartyNotices.txt (different order)

* Also add Support/bin to .pre-commit-config

* Ignore duplicate-code

* Also fix/ignore pylint and flake8 on Support/bin

* Create main.yml

.flake8

@@ -0,0 +1,2 @@
+[flake8]
+extend-ignore = E111, E114, E501, E722, E121

.github/workflows/main.yml

@@ -0,0 +1,14 @@
+name: pre-commit
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+    - uses: actions/checkout@v2
+    - uses: actions/setup-python@v2
+    - uses: pre-commit/[email protected]

.pre-commit-config.yaml

@@ -0,0 +1,16 @@
+---
+repos:
+  - repo: https://github.com/pre-commit/pre-commit-hooks
+    rev: v2.0.0
+    hooks:
+      - id: flake8
+        files: ^Support/
+  - repo: https://github.com/PyCQA/pylint.git
+    rev: pylint-2.6.0
+    hooks:
+      - id: pylint
+        name: pylint
+        files: ^Support/
+        language_version: python3
+        args:
+          - --load-plugins=pylint.extensions.redefined_variable_type,pylint.extensions.bad_builtin

.pylintrc

@@ -0,0 +1,2 @@
+[MESSAGES CONTROL]
+disable=bad-indentation,missing-class-docstring,missing-module-docstring,missing-function-docstring,invalid-name,fixme,line-too-long,duplicate-code

Support/Python/tbdata/brush_lookup.py

@@ -16,18 +16,20 @@
 import re
 from collections import defaultdict
 
-class BrushLookup(object):
+
+class BrushLookup():
   """Helper for doing name <-> guid conversions for brushes."""
   @staticmethod
   def iter_brush_guid_and_name(tilt_brush_dir):
     for brush_dir in ("Assets/Resources/Brushes", "Assets/Resources/X/Brushes"):
-      for r, ds, fs in os.walk(os.path.join(tilt_brush_dir, brush_dir)):
+      for r, _, fs in os.walk(os.path.join(tilt_brush_dir, brush_dir)):
         for f in fs:
           if f.lower().endswith('.asset'):
             fullf = os.path.join(r, f)
-            data = file(fullf).read()
+            with open(fullf) as f:
+              data = f.read()
             guid = re.search('m_storage: (.*)$', data, re.M).group(1)
-            name = re.search('m_Name: (.*)$', data, re.M).group(1)
+            # name = re.search('m_Name: (.*)$', data, re.M).group(1)
 
             yield guid, f[:-6]
 
@@ -48,10 +50,10 @@ def __init__(self, tilt_brush_dir):
     self.initialized = True
     self.guid_to_name = dict(self.iter_brush_guid_and_name(tilt_brush_dir))
     # Maps name -> list of guids
-    self.name_to_guids = defaultdict(list)
+    name_to_guids = defaultdict(list)
     for guid, name in self.guid_to_name.items():
-      self.name_to_guids[name].append(guid)
-    self.name_to_guids = dict(self.name_to_guids)
+      name_to_guids[name].append(guid)
+    self.name_to_guids = dict(name_to_guids)
 
   def get_unique_guid(self, name):
     lst = self.name_to_guids[name]

Support/Python/tbdata/bvh.py

@@ -16,9 +16,9 @@
 # Usage:
 #   RTree.from_bounds_iter()
 
-import os
 import struct
 import sys
+from collections import deque
 from io import StringIO
 
 try:
@@ -37,21 +37,21 @@ class BBox(tuple):
   def union(lhs, rhs):
     lmin, lmax = lhs
     rmin, rmax = rhs
-    nmin = ( min(lmin[0], rmin[0]),
-             min(lmin[1], rmin[1]),
-             min(lmin[2], rmin[2]) )
-    nmax = ( max(lmax[0], rmax[0]),
-             max(lmax[1], rmax[1]),
-             max(lmax[2], rmax[2]) )
+    nmin = (min(lmin[0], rmin[0]),
+            min(lmin[1], rmin[1]),
+            min(lmin[2], rmin[2]))
+    nmax = (max(lmax[0], rmax[0]),
+            max(lmax[1], rmax[1]),
+            max(lmax[2], rmax[2]))
     for i in range(3):
-      assert nmax[i] > nmin[i], self
+      assert nmax[i] > nmin[i]
     return BBox((nmin, nmax))
 
   def half_width(self):
     bmin, bmax = self
-    return ((bmax[0]-bmin[0]) * .5,
-            (bmax[1]-bmin[1]) * .5,
-            (bmax[2]-bmin[2]) * .5)
+    return ((bmax[0] - bmin[0]) * .5,
+            (bmax[1] - bmin[1]) * .5,
+            (bmax[2] - bmin[2]) * .5)
 
   def surface_area(self):
     dx, dy, dz = self.half_width()
@@ -63,21 +63,24 @@ def surface_area(self):
 # RTree
 # ---------------------------------------------------------------------------
 
+
 # Tunables
 INDEX_CAPACITY = 80
 LEAF_CAPACITY = 410
 
+
 def str_vec3(vec3):
   return "(%6.1f %6.1f %6.1f)" % vec3
 
+
 def str_bounds(bounds):
-  #return "(%s, %s)" % (str_vec3(bounds[0]), str_vec3(bounds[1]))
+  # return "(%s, %s)" % (str_vec3(bounds[0]), str_vec3(bounds[1]))
   bmin, bmax = bounds
-  halfsize = (bmax[0]-bmin[0], bmax[1]-bmin[1], bmax[2]-bmin[2])
+  halfsize = (bmax[0] - bmin[0], bmax[1] - bmin[1], bmax[2] - bmin[2])
   return "(%5.1f %5.1f %5.1f)" % halfsize
 
 
-class BinaryReader(object):
+class BinaryReader():
   # Wraps struct.unpack
   def __init__(self, inf):
     if isinstance(inf, bytes):
@@ -104,9 +107,11 @@ def __init__(self):
   def flush(self, returnError):
     # print "flush"
     pass
+
   def create(self, returnError):
     # print "create"
     pass
+
   def destroy(self, returnError):
     # print "destroy"
     pass
@@ -120,25 +125,26 @@ def loadByteArray(self, page, returnError):
     except KeyError:
       returnError.contents.value = self.InvalidPageError
 
-  def storeByteArray(self, page, data, returnError):
+  def storeByteArray(self, page, data):
     if page == self.NewPage:
       page = len(self.datas)
       self.datas[page] = data
       # print "store new %s %s" % (page, len(data))
     else:
       assert page in self.datas
-      old_data = self.datas[page]
+      # old_data = self.datas[page]
       self.datas[page] = data
       # print "store %s %s -> %s" % (page, len(old_data), len(data))
 
     return page
 
-  def deleteByteArray(self, page, returnError):
+  def deleteByteArray(self, page):
     # print "RTreeStorageDict: delete", page
     assert page in self.datas
     self.datas[page] = 'deleted'
-
-class RTree(object):
+
+
+class RTree():
   @classmethod
   def from_bounds_iter(cls, bounds_iter, leaf_capacity_multiplier=1):
     storage = RTreeStorageDict()
@@ -168,11 +174,10 @@ def _recursive_create_node(self, storage, node_id):
       for c in node.children:
         assert c.data is None
         c.node = self._recursive_create_node(storage, c.id)
-      
+
     return node
 
   def dfs_iter(self):
-    from collections import deque
     q = deque([self.root])
     while q:
       n = q.popleft()
@@ -181,7 +186,7 @@ def dfs_iter(self):
         q.extend(c.node for c in n.children)
 
 
-class RTreeHeader(object):
+class RTreeHeader():  # pylint: disable=too-few-public-methods,too-many-instance-attributes
   # RTree::storeHeader
   #  id_type    root
   #  u32        RTreeVariant (0 linear, 1 quadratic, 2 rstar)
@@ -220,7 +225,9 @@ def as_str(self):
 
 class CannotSplit(Exception):
   pass
-class RTreeNode(object):
+
+
+class RTreeNode():
   # for nodeType
   PERSISTENT_INDEX = 1
   PERSISTENT_LEAF = 2
@@ -240,7 +247,7 @@ def __init__(self, node_id, inf_or_data):
     self.node_id = node_id
     reader = BinaryReader(inf_or_data)
     (self.nodeType, self.level, nChildren) = reader.read("III")
-    self.children = [ RTreeChild(reader) for i in range(nChildren) ]
+    self.children = [RTreeChild(reader) for i in range(nChildren)]
     self.bounds = reader.read_bounds()
 
   def is_index(self):
@@ -267,6 +274,7 @@ def resplit(self, multiplier):
     #   isinstance(v, RTreeChild) == True
     #   v.node.is_leaf() == True
     bounds = []
+
     def get_all_bounds(node, bounds):
       if node.is_leaf():
         for c in node.children:
@@ -279,14 +287,16 @@ def get_all_bounds(node, bounds):
     new_node = RTree.from_bounds_iter(bounds, multiplier).root
     if new_node.is_leaf():
       raise CannotSplit()
+
     def iter_leaf_children(node):
       for c in node.children:
         if c.node.is_leaf():
           c.id = c.node.node_id = RTreeNode.GENERATED_SPLIT_ID
           RTreeNode.GENERATED_SPLIT_ID -= 1
           yield c
         else:
-          for leaf in iter_leaf_children(c.node):
+          for leaf in iter_leaf_children(c.node):  # pylint: disable=unused-variable
+            # TODO: Should this be "yield leaf"? (if so, remove the pylint comment above)
             yield c
 
     lst = list(iter_leaf_children(new_node))
@@ -301,7 +311,7 @@ def as_str(self):
       str_bounds(self.bounds))
 
 
-class RTreeChild(object):
+class RTreeChild():  # pylint: disable=too-few-public-methods
   # .id
   #   If parent is PERSISTENT_INDEX, this is a node id.
   #   If parent is PESISTENT_LEAF, this is some leaf-specific id (like a stroke number)