Use pre-commit for linting in GitHub Actions Workflow (#254)

* Update flake8 repo to use github instead of gitlab

* pylint: update code based on errors

* Update repo versions in pre-commit-config

And update fil load methods based on error detected by pylint

* Remove tox lint environment

* Add lint workflow to run pre-commit on all files

.github/workflows/cpu-ci.yml

@@ -33,9 +33,6 @@ jobs:
     - name: Install and upgrade python packages
       run: |
         python -m pip install --upgrade pip setuptools==59.4.0 wheel tox
-    - name: Lint with flake8, black, isort, interrogate, codespell
-      run: |
-        tox -e lint
     - name: Run tests
       run: |
         ref_type=${{ github.ref_type }}

.github/workflows/lint.yaml

@@ -0,0 +1,16 @@
+name: lint
+
+on:
+  pull_request:
+  push:
+    branches: [main]
+    tags:
+      - v*
+
+jobs:
+  pre-commit:
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v3
+      - uses: actions/setup-python@v4
+      - uses: pre-commit/[email protected]

.pre-commit-config.yaml

@@ -1,20 +1,20 @@
 repos:
       - repo: https://github.com/timothycrosley/isort
-        rev: 5.10.1
+        rev: 5.11.2
         hooks:
         - id: isort
           additional_dependencies: [toml]
           exclude: examples/.*
       - repo: https://github.com/python/black
-        rev: 22.6.0
+        rev: 22.12.0
         hooks:
         - id: black
-      - repo: https://gitlab.com/pycqa/flake8
-        rev: 3.9.2
+      - repo: https://github.com/pycqa/flake8
+        rev: 6.0.0
         hooks:
         - id: flake8
       - repo: https://github.com/pycqa/pylint
-        rev: v2.14.5
+        rev: v2.15.8
         hooks:
         - id: pylint
       - repo: https://github.com/econchick/interrogate
@@ -24,7 +24,7 @@ repos:
           exclude: ^(docs|tests|setup.py|versioneer.py)
           args: [--config=pyproject.toml]
       - repo: https://github.com/codespell-project/codespell
-        rev: v2.1.0
+        rev: v2.2.2
         hooks:
         - id: codespell
       - repo: https://github.com/PyCQA/bandit

.pylintrc

@@ -22,7 +22,6 @@ disable=fixme,
     # we'll probably never enable these checks
     invalid-name,
     import-error,
-    no-self-use,
 
     # disable code-complexity checks for now
     # TODO: should we configure the thresholds for these rather than just disable?

merlin/systems/dag/ops/fil.py

@@ -286,7 +286,7 @@ def export(
 
     def load_model(self, version_path):
         version_path = pathlib.Path(version_path)
-        self.fil_model_class.model = self.fil_model_class.load(version_path)
+        self.fil_model_class.load(version_path)
 
     def predict(self, inputs):
         return self.fil_model_class.predict(inputs)
@@ -441,6 +441,7 @@ def load(self, version_path) -> "XGBoost":
             self.model = cuml_fil.load(model_path, output_class=True)
         else:
             self.model = treelite_model.load(model_path, self.model_type)
+        return self.model
 
     def _get_learner(self):
         return self._learner
@@ -482,6 +483,7 @@ def load(self, version_path) -> "LightGBM":
             self.model = cuml_fil.load(model_path, output_class=True)
         else:
             self.model = treelite_model.load(model_path, self.model_type)
+        return self.model
 
     @property
     def num_features(self):
@@ -522,6 +524,7 @@ def load(self, version_path) -> "SKLearnRandomForest":
                 "are required to save an sklearn random forest model."
             )
         self.model = treelite_model.deserialize(str(model_path))
+        return self.model
 
     @property
     def num_features(self):
@@ -554,9 +557,10 @@ def load(self, version_path) -> "CUMLRandomForest":
         """Load model to version_path."""
         model_path = pathlib.Path(version_path) / self.model_filename
         if HAS_GPU:
-            self.model = cuml_fil.load(model_path, output_class=True)
+            model = cuml_fil.load(model_path, output_class=True)
         else:
-            self.model = treelite_model.deserialize(model_path)
+            model = treelite_model.deserialize(model_path)
+        return model
 
     @property
     def num_features(self):

merlin/systems/dag/runtimes/triton/ops/operator.py

@@ -29,6 +29,7 @@ def __init__(self, base_op: InferenceOperator):
         base_op : merlin.systems.dag.ops.operator.InfereneOperator
             Base operator used to construct this Triton op.
         """
+        super().__init__()
         self.op = base_op
 
     @property

merlin/systems/workflow/hugectr.py

@@ -79,7 +79,7 @@ def get_offsets(self, workflow, categorical_cols):
         if embeddings is None:
             raise Exception("embeddings cannot be None")
         else:
-            offsets = dict()
+            offsets = {}
             curr_offset = 0
             for name in categorical_cols:
                 offsets[name] = curr_offset

requirements/dev.txt

@@ -1,14 +1,2 @@
 # Required to generate docs and also run tests
 tritonclient[all]
-
-# linting
-pylint==2.7.4
-black==22.6.0
-flake8==3.9.2
-isort==5.9.3
-bandit==1.7.0
-flake8-nb==0.3.0
-cpplint>=1.5
-codespell
-interrogate==1.5.0
-click<8.1.0

tox.ini

@@ -60,20 +60,6 @@ commands =
     ; this runs the tests then removes the Merlin repo directory whether the tests work or fail
     python -m pytest --cov merlin --cov-report term Merlin-{env:GIT_COMMIT}/tests/unit
 
-[testenv:lint]
-; Runs in: Github Actions
-; Runs all lint/code checks and fails the PR if there are errors.
-; Install pre-commit-hooks to run these tests during development.
-deps = -rrequirements/dev.txt
-commands =
-    python -m pip install --upgrade git+https://github.com/NVIDIA-Merlin/core.git
-    flake8 setup.py merlin/ tests/
-    black --check --diff merlin tests
-    pylint merlin
-    isort -c . --skip .tox
-    interrogate --config=pyproject.toml
-    codespell --skip .tox
-
 [testenv:docs]
 ; Runs in: Github Actions
 ; Generates documentation with sphinx. There are other steps in the Github Actions workflow

versioneer.py

@@ -1,3 +1,4 @@
+# pylint: skip-file
 # Version: 0.23
 
 """The Versioneer - like a rocketeer, but for versions.