Linting and formatting, semi-lax. (#172)

* Linting and formatting, semi-lax.

  - `tox` now runs `black` and `ruff` on the project (the environment is called `lint`).
  - tox runs `ruff` with --fix to resolve fixable issues.
  - There is no pre-commit hook or CI integration as of yet.

### Code changes

To adhere to even semi-lax linting, there were changes required in the code. These are:

- Migrated use of python builtin identifiers (`type`,  `id`, and `print`) to more appropriate names.
- Some cases of long string literals were using implicit concatenation, which is now explicit (by adding a set of parentheses).

### Outstanding changes with current settings

- `print` in history.py (shadowing builtin) was not fixed as it needs a bit more debugging to figure out exactly how it is used.

### Notes

There is still a lot to do with regards to linting, formatting, and typing. This PR does not seek to resolve these issues, but attempts to create a baseline of requirements for the codebase to adhere to.

Author: terjekv

.github/workflows/test.yml

@@ -7,7 +7,7 @@ jobs:
     runs-on: ${{ matrix.os }}
     strategy:
       matrix:
-        os: [ubuntu-20.04]
+        os: [ubuntu-20.04] # For python3.6
         python-version:
           - '3.6'
           - '3.7'
@@ -37,3 +37,32 @@ jobs:
           pip install -e .
       - name: Test and compare api calls
         run: ci/run_testsuite_and_record.sh
+
+  tox:
+    name: tox
+    runs-on: ubuntu-latest
+    strategy:
+      fail-fast: false
+      matrix:
+        python-version:
+          - "3.7"
+          - "3.8"
+          - "3.9"
+          - "3.10"
+          - "3.11"
+
+    steps:
+      - uses: actions/checkout@v3
+      - name: Set up Python ${{ matrix.python-version }}
+        uses: actions/setup-python@v4
+        with:
+          python-version: ${{ matrix.python-version }}
+      - name: Install dependencies
+        run: |
+          python -m pip install --upgrade pip
+          python -m pip install tox tox-gh-actions
+          python -m pip install -r requirements.txt
+          python -m pip install -r requirements-dev.txt
+      - name: Test with tox
+        run: tox r
+        

ci/diff.py

@@ -2,8 +2,9 @@
 import json
 import sys
 
+
 def group_objects(json_file_path):
-    with open(json_file_path, 'r') as f:
+    with open(json_file_path, "r") as f:
         data = json.load(f)
 
     grouped_objects = []
@@ -23,53 +24,57 @@ def group_objects(json_file_path):
 
 
 def main():
+    if len(sys.argv) != 3:
+        print("Usage: diff.py <file1> <file2>")
+        sys.exit(1)
+
+    expected = group_objects(sys.argv[1])
+    result = group_objects(sys.argv[2])
 
-	if len(sys.argv) != 3:
-		print("Usage: diff.py <file1> <file2>")
-		sys.exit(1)
+    # Verify that the list of commands is the same
+    cmdlist1 = []
+    cmdlist2 = []
+    for a in expected:
+        cmdlist1.append(a[0]["command"].rstrip())
+    for a in result:
+        cmdlist2.append(a[0]["command"].rstrip())
+    differ = difflib.Differ()
+    diff = differ.compare(cmdlist1, cmdlist2)
+    differences = [
+        line for line in diff if line.startswith("-") or line.startswith("+")
+    ]
+    if differences:
+        print(
+            "Diff between what commands were run in the recorded result and the current testsuite:"
+        )
+        for line in differences:
+            print(line)
+        sys.exit(1)
 
-	expected = group_objects(sys.argv[1])
-	result = group_objects(sys.argv[2])
+    # For each command, verify that the http calls and output is the same
+    has_diff = False
+    for i in range(len(expected)):
+        cmd = expected[i][0]["command"].rstrip()
+        cmd2 = result[i][0]["command"].rstrip()
+        if cmd != cmd2:
+            # This should never happen here, because it would get caught above
+            print(f"Expected command: {cmd}\nActual command: {cmd2}")
+            sys.exit(1)
 
-	# Verify that the list of commands is the same	
-	cmdlist1 = []
-	cmdlist2 = []
-	for a in expected:
-		cmdlist1.append(a[0]['command'].rstrip())
-	for a in result:
-		cmdlist2.append(a[0]['command'].rstrip())
-	differ = difflib.Differ()
-	diff = differ.compare(cmdlist1, cmdlist2)
-	differences = [line for line in diff if line.startswith('-') or line.startswith('+')]
-	if differences:
-		print("Diff between what commands were run in the recorded result and the current testsuite:")
-		for line in differences:
-			print(line)
-		sys.exit(1)
+        s1 = json.dumps(expected[i], indent=4).splitlines(keepends=True)
+        s2 = json.dumps(result[i], indent=4).splitlines(keepends=True)
+        if s1 != s2:
+            has_diff = True
+            print("=" * 72)
+            print("Command:", cmd, "       -expected, +tested")
+            print("=" * 72)
+            gen = difflib.ndiff(s1, s2)
+            sys.stdout.writelines(gen)
+            print("\n")  # 2 newlines
 
-	# For each command, verify that the http calls and output is the same
-	has_diff = False
-	for i in range(len(expected)):
-		cmd = expected[i][0]['command'].rstrip()
-		cmd2 = result[i][0]['command'].rstrip()
-		if cmd != cmd2:
-			# This should never happen here, because it would get caught above
-			print(f"Expected command: {cmd}\nActual command: {cmd2}")
-			sys.exit(1)
-		
-		s1 = json.dumps(expected[i], indent=4).splitlines(keepends=True)
-		s2 = json.dumps(result[i], indent=4).splitlines(keepends=True)
-		if s1 != s2:
-			has_diff = True
-			print("=" * 72)
-			print("Command:",cmd,"       -expected, +tested")
-			print("=" * 72)
-			gen = difflib.ndiff(s1,s2)
-			sys.stdout.writelines(gen)
-			print("\n") # 2 newlines
+    if has_diff:
+        sys.exit(1)
 
-	if has_diff:
-		sys.exit(1)
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     main()

mreg_cli/__init__.pyi

@@ -1 +1 @@
-from .log import *
+from .log import *

mreg_cli/__main__.py

@@ -1,4 +1,4 @@
 from . import main
 
-if __name__ == '__main__':
+if __name__ == "__main__":
     main.main()

mreg_cli/bacnet.py

@@ -1,71 +1,69 @@
-from .cli import Flag, cli
-from .exceptions import HostNotFoundWarning
+from .cli import Flag
 from .history import history
 from .host import host
-from .log import cli_error, cli_info, cli_warning
-from .util import (
-    host_info_by_name,
-    print_table,
-    get,
-    get_list,
-    post,
-    delete,
-)
+from .log import cli_error, cli_info
+from .util import delete, get, get_list, host_info_by_name, post, print_table
+
 
 def bacnetid_add(args):
     info = host_info_by_name(args.name)
-    if 'bacnetid' in info and info['bacnetid'] is not None:
-        cli_error("{} already has BACnet ID {}.".format(info['name'],info['bacnetid']['id']))
-    postdata = {'hostname': info['name']}
-    path = '/api/v1/bacnet/ids/'
-    bacnetid = getattr(args, 'id')
+    if "bacnetid" in info and info["bacnetid"] is not None:
+        cli_error(
+            "{} already has BACnet ID {}.".format(info["name"], info["bacnetid"]["id"])
+        )
+    postdata = {"hostname": info["name"]}
+    path = "/api/v1/bacnet/ids/"
+    bacnetid = args.id
     if bacnetid:
-        response = get(path+bacnetid, ok404=True)
+        response = get(path + bacnetid, ok404=True)
         if response:
             j = response.json()
-            cli_error('BACnet ID {} is already in use by {}'.format(j['id'], j['hostname']))
-        postdata['id'] = bacnetid
-    history.record_post(path, '', postdata)
+            cli_error(
+                "BACnet ID {} is already in use by {}".format(j["id"], j["hostname"])
+            )
+        postdata["id"] = bacnetid
+    history.record_post(path, "", postdata)
     post(path, **postdata)
     info = host_info_by_name(args.name)
-    if 'bacnetid' in info and info['bacnetid'] is not None:
-        b = info['bacnetid']
-        cli_info("Assigned BACnet ID {} to {}".format(b['id'], info['name']), print_msg=True)
+    if "bacnetid" in info and info["bacnetid"] is not None:
+        b = info["bacnetid"]
+        cli_info(
+            "Assigned BACnet ID {} to {}".format(b["id"], info["name"]), print_msg=True
+        )
 
 
 host.add_command(
-    prog='bacnetid_add',
-    description='Assign a BACnet ID to the host.',
-    short_desc='Add BACnet ID',
+    prog="bacnetid_add",
+    description="Assign a BACnet ID to the host.",
+    short_desc="Add BACnet ID",
     callback=bacnetid_add,
     flags=[
-        Flag('name',
-             description='Name of host.',
-             metavar='NAME'),
-        Flag('-id',
-             description='ID value (0-4194302)',
-             metavar='ID'),
+        Flag("name", description="Name of host.", metavar="NAME"),
+        Flag("-id", description="ID value (0-4194302)", metavar="ID"),
     ],
 )
 
+
 def bacnetid_remove(args):
     info = host_info_by_name(args.name)
-    if 'bacnetid' not in info or info["bacnetid"] is None:
-        cli_error("{} does not have a BACnet ID assigned.".format(info['name']))
-    path = '/api/v1/bacnet/ids/{}'.format(info['bacnetid']['id'])
-    history.record_delete(path, info['bacnetid'])
+    if "bacnetid" not in info or info["bacnetid"] is None:
+        cli_error("{} does not have a BACnet ID assigned.".format(info["name"]))
+    path = "/api/v1/bacnet/ids/{}".format(info["bacnetid"]["id"])
+    history.record_delete(path, info["bacnetid"])
     delete(path)
-    cli_info("Unassigned BACnet ID {} from {}".format(info['bacnetid']['id'], info['name']), print_msg=True)
+    cli_info(
+        "Unassigned BACnet ID {} from {}".format(info["bacnetid"]["id"], info["name"]),
+        print_msg=True,
+    )
+
 
 host.add_command(
-    prog='bacnetid_remove',
-    description='Unassign the BACnet ID from the host.',
-    short_desc='Remove BACnet ID',
+    prog="bacnetid_remove",
+    description="Unassign the BACnet ID from the host.",
+    short_desc="Remove BACnet ID",
     callback=bacnetid_remove,
     flags=[
-        Flag('name',
-             description='Name of host.',
-             metavar='NAME'),
+        Flag("name", description="Name of host.", metavar="NAME"),
     ],
 )
 
@@ -81,22 +79,27 @@ def bacnetid_list(args):
         maxval = args.max
         if maxval > 4194302:
             cli_error("The maximum ID value is 4194302.")
-    r = get_list("/api/v1/bacnet/ids/",{'id__range':'{},{}'.format(minval,maxval)})
-    print_table(('ID','Hostname'), ('id','hostname'), r)
+    r = get_list("/api/v1/bacnet/ids/", {"id__range": "{},{}".format(minval, maxval)})
+    print_table(("ID", "Hostname"), ("id", "hostname"), r)
+
 
 host.add_command(
-    prog='bacnetid_list',
-    description='Find/list BACnet IDs and hostnames',
-    short_desc='List used BACnet IDs',
+    prog="bacnetid_list",
+    description="Find/list BACnet IDs and hostnames",
+    short_desc="List used BACnet IDs",
     callback=bacnetid_list,
     flags=[
-        Flag('-min',
-             description='Minimum ID value (0-4194302)',
-             type=int,
-             metavar='MIN'),
-        Flag('-max',
-             description='Maximum ID value (0-4194302)',
-             type=int,
-             metavar='MAX'),
+        Flag(
+            "-min",
+            description="Minimum ID value (0-4194302)",
+            flag_type=int,
+            metavar="MIN",
+        ),
+        Flag(
+            "-max",
+            description="Maximum ID value (0-4194302)",
+            flag_type=int,
+            metavar="MAX",
+        ),
     ],
 )