Fix #62: add verbose option (#65)

* Report configuration used.
* Report which units checked for each file.
* Report total issues found, total files processed, total files not processed.
* Improve messages to user.
* Update tests and docs.

CHANGELOG.md

@@ -11,6 +11,9 @@ This project does *not* adhere to [Semantic Versioning](https://semver.org).
 ## [Unreleased](https://github.com/dsa-ou/allowed/compare/v1.3.0...HEAD)
 These changes are in the GitHub repository but not on [PyPI](https://pypi.org/project/allowed).
 
+### Added
+- option `-v`/ `--verbose`: display additional info about the checking process
+
 ### Fixed
 - report syntax errors in notebooks in the same way as in Python files
 

allowed/allowed.py

@@ -10,6 +10,11 @@
 import sys
 from pathlib import Path
 
+issues = 0  # number of issues (unknown constructs) found
+py_checked = 0  # number of Python files checked
+nb_checked = 0  # number of notebooks checked
+unchecked = 0  # number of .py and .ipynb files skipped due to syntax or other errors
+
 try:
     import pytype
     from pytype.tools.annotate_ast import annotate_ast
@@ -283,6 +288,17 @@ def check_imports() -> str:
 # ----- auxiliary functions -----
 
 
+def show_units(filename: str, last_unit: int) -> None:
+    """Print a message about the units being checked."""
+    if last_unit == 1:
+        units = "unit 1"
+    elif last_unit > 0:
+        units = f"units 1–{last_unit}"  # noqa: RUF001 (it's an en-dash)
+    else:
+        units = "all units"
+    print(f"INFO: checking {filename} against {units}")
+
+
 def get_unit(filename: str) -> int:
     """Return the file's unit or zero (consider all units)."""
     if FILE_UNIT and (match := re.match(FILE_UNIT, filename)):
@@ -389,9 +405,6 @@ def ignore(node: ast.AST, source: list[str]) -> bool:
 
 # ----- main functions -----
 
-read_nb = False  # remember if a notebook was read
-read_py = False  # remember if Python file was read
-
 
 def check_tree(
     tree: ast.AST,
@@ -482,18 +495,23 @@ def check_folder(
     last_unit: int,
     check_method_calls: bool,  # noqa: FBT001
     report_first: bool,  # noqa: FBT001
+    verbose: bool,  # noqa: FBT001
 ) -> None:
     """Check all Python files in `folder` and its subfolders."""
     global_constructs = get_constructs(last_unit)
     for current_folder, subfolders, files in os.walk(folder):
         subfolders.sort()
         for filename in sorted(files):
             if filename.endswith((".py", ".ipynb")):
+                fullname = str(Path(current_folder) / filename)
                 if not last_unit and (file_unit := get_unit(filename)):
                     constructs = get_constructs(file_unit)
+                    if verbose:
+                        show_units(fullname, file_unit)
                 else:
                     constructs = global_constructs
-                fullname = str(Path(current_folder) / filename)
+                    if verbose:
+                        show_units(fullname, last_unit)
                 check_file(fullname, constructs, check_method_calls, report_first)
 
 
@@ -504,45 +522,55 @@ def check_file(
     report_first: bool,  # noqa: FBT001
 ) -> None:
     """Check that the file only uses the allowed constructs."""
-    global read_nb, read_py
+    global py_checked, nb_checked, unchecked, issues
 
     try:
         with Path(filename).open(encoding="utf-8", errors="surrogateescape") as file:
             if filename.endswith(".ipynb"):
                 source, line_cell_map, errors = read_notebook(file.read())
-                read_nb = True
             else:
                 source = file.read()
                 line_cell_map = []
                 errors = []
-                read_py = True
         if check_method_calls and METHODS:
             tree = annotate_ast.annotate_source(source, ast, PYTYPE_OPTIONS)
         else:
             tree = ast.parse(source)
         check_tree(tree, constructs, source.splitlines(), line_cell_map, errors)
         errors.sort()
-        messages = set()  # for --first option: unique messages (except errors)
+        messages = set()  # for --first option: the unique messages (except errors)
         last_error = None
         for cell, line, message in errors:
             if (cell, line, message) != last_error and message not in messages:
                 if cell:
                     print(f"{filename}:cell_{cell}:{line}: {message}")
                 else:
                     print(f"{filename}:{line}: {message}")
+                # don't count syntax errors as unknown constructs
+                if "ERROR" not in message:
+                    issues += 1
                 if report_first and "ERROR" not in message:
                     messages.add(message)
                 last_error = (cell, line, message)
+        if filename.endswith(".py"):
+            py_checked += 1
+        else:
+            nb_checked += 1
     except OSError as error:
         print(f"{filename}: OS ERROR: {error.strerror}")
+        unchecked += 1
     except SyntaxError as error:
         print(f"{filename}:{error.lineno}: SYNTAX ERROR: {error.msg}")
+        unchecked += 1
     except UnicodeError as error:
         print(f"{filename}: UNICODE ERROR: {error}")
+        unchecked += 1
     except json.decoder.JSONDecodeError as error:
         print(f"{filename}:{error.lineno}: FORMAT ERROR: invalid notebook format")
+        unchecked += 1
     except ValueError as error:
         print(f"{filename}: VALUE ERROR: {error}")
+        unchecked += 1
     except annotate_ast.PytypeError as error:
         #  write 'file:n: error' instead of 'Error reading file ... at line n: error'
         message = str(error)
@@ -552,6 +580,7 @@ def check_file(
             print(f"{filename}:{line}: PYTYPE ERROR: {message}")
         else:
             print(f"{filename}: PYTYPE ERROR: {message}")
+        unchecked += 1
 
 
 def read_notebook(file_contents: str) -> tuple[str, list, list]:
@@ -627,6 +656,12 @@ def main() -> None:
         default="m269.json",
         help="allow the constructs given in CONFIG (default: m269.json)",
     )
+    argparser.add_argument(
+        "-v",
+        "--verbose",
+        action="store_true",
+        help="show additional info as files are processed",
+    )
     argparser.add_argument("file_or_folder", nargs="+", help="file or folder to check")
     args = argparser.parse_args()
 
@@ -642,6 +677,8 @@ def main() -> None:
             if file.exists():
                 with file.open() as config_file:
                     configuration = json.load(config_file)
+                    if args.verbose:
+                        print(f"INFO: using configuration {file.resolve()}")
                     break
         else:
             print(f"CONFIGURATION ERROR: {args.config} not found")
@@ -674,24 +711,48 @@ def main() -> None:
 
     for name in args.file_or_folder:
         if Path(name).is_dir():
-            check_folder(name, args.unit, args.methods, args.first)
+            check_folder(name, args.unit, args.methods, args.first, args.verbose)
         elif name.endswith((".py", ".ipynb")):
             unit = args.unit if args.unit else get_unit(Path(name).name)
+            if args.verbose:
+                show_units(name, unit)
             check_file(name, get_constructs(unit), args.methods, args.first)
         else:
             print(f"WARNING: {name} skipped: not a folder, Python file or notebook")
 
-    if (read_py or read_nb) and not args.methods:
+    if args.verbose:
+        print(
+            "INFO: checked",
+            f"{py_checked} Python file{'' if py_checked == 1 else 's'} and",
+            f"{nb_checked} notebook{'' if nb_checked == 1 else 's'}",
+        )
+        if issues:
+            print(
+                f"INFO: the {issues} Python construct{'s' if issues > 1 else ''}",
+                f"listed above {'are' if issues > 1 else 'is'} not allowed",
+            )
+        elif nb_checked or py_checked:
+            print("INFO: found no disallowed Python constructs")
+        if unchecked:
+            print(
+                f"INFO: didn't check {unchecked} Python",
+                f"file{'s' if unchecked > 1 else ''} or notebook{'s' if unchecked > 1 else ''}",
+                "due to syntax or other errors",
+            )
+    if args.first and issues:
+        print(
+            "WARNING:",
+            "other occurrences of the listed constructs may exist (don't use option -f)",  # noqa: E501
+        )
+    if (py_checked or nb_checked) and not args.methods:
         print(
             "WARNING: didn't check method calls",
             "(use option -m)" if PYTYPE_INSTALLED else "(pytype not installed)",
         )
-    if read_nb and not IPYTHON_INSTALLED:
+    if nb_checked and not IPYTHON_INSTALLED:
         print(
             "WARNING: didn't check notebook cells with %-commands (IPython not installed)"  # noqa: E501
         )
-    if (read_py or read_nb) and args.first:
-        print("WARNING: each construct was reported once; other occurrences may exist")
 
 
 if __name__ == "__main__":

docs/usage.md

@@ -67,6 +67,11 @@ was _not_ checked, for these reasons:
 - `UNICODE ERROR`: the file has some strange characters and couldn't be read
 - `VALUE ERROR`: some other cause; please report it to us.
 
+When the command line option `-v` or `--verbose` is given,
+the tool outputs additional information, including
+the total number of files processed and of unknown constructs found, and
+the total number of files not processed due to syntax, format or other errors.
+
 ### Extra checks
 
 To check method calls of the form `variable.method(...)`,
@@ -115,7 +120,7 @@ allowed --unit 5 01_submission.py
 ### Checking notebooks
 
 As mentioned earlier, `allowed` does check Jupyter notebooks and
-reports the cells and lines with disallowed constructs: `notebook.ipynb:cell_13:5: ...`
+reports the cells and lines with unknown constructs: `notebook.ipynb:cell_13:5: ...`
 means that line 5 of the 13th code cell uses a construct that wasn't taught.
 
 If a code cell has invalid Python, `allowed` reports a syntax error and
@@ -127,4 +132,7 @@ if it hasn't other syntax errors.
 The transformed commands use function calls and attributes, so
 the cell will only pass the check if those Python constructs are allowed.
 
+In the verbose output (option `-v` / `--verbose`), syntax errors do not count
+towards the unknown constructs total.
+
 ⇦ [Installation](installation.md) | ⇧ [Start](../README.md) | [Configuration](configuration.md) ⇨

tests/folder-first.txt

@@ -1,3 +1,5 @@
+INFO: using configuration /Users/mw4687/GitHub/dsa-ou/allowed/allowed/m269.json
+INFO: checking tests/sample.ipynb against all units
 tests/sample.ipynb:cell_1:2: SYNTAX ERROR: '(' was never closed
 tests/sample.ipynb:cell_2:4: types
 tests/sample.ipynb:cell_2:5: choice
@@ -12,30 +14,35 @@ tests/sample.ipynb:cell_6:4: f-string
 tests/sample.ipynb:cell_6:9: <<
 tests/sample.ipynb:cell_6:10: math.e
 tests/sample.ipynb:cell_6:11: type()
+INFO: checking allowed/__init__.py against all units
+INFO: checking allowed/__main__.py against all units
 allowed/__main__.py:1: allowed
+INFO: checking allowed/allowed.py against all units
 allowed/allowed.py:5: argparse
 allowed/allowed.py:6: ast
 allowed/allowed.py:7: json
 allowed/allowed.py:8: os
 allowed/allowed.py:9: re
 allowed/allowed.py:10: sys
 allowed/allowed.py:11: pathlib
-allowed/allowed.py:13: try
-allowed/allowed.py:14: pytype
-allowed/allowed.py:15: pytype.tools.annotate_ast
-allowed/allowed.py:22: IPython.core.inputtransformer2
-allowed/allowed.py:123: dict comprehension
-allowed/allowed.py:259: if expression
-allowed/allowed.py:276: f-string
-allowed/allowed.py:288: :=
-allowed/allowed.py:289: int()
-allowed/allowed.py:385: hasattr()
-allowed/allowed.py:407: isinstance()
-allowed/allowed.py:412: type()
-allowed/allowed.py:507: global
-allowed/allowed.py:510: with
-allowed/allowed.py:645: break
-allowed/allowed.py:646: for-else
-allowed/allowed.py:653: raise
+allowed/allowed.py:18: try
+allowed/allowed.py:19: pytype
+allowed/allowed.py:20: pytype.tools.annotate_ast
+allowed/allowed.py:27: IPython.core.inputtransformer2
+allowed/allowed.py:128: dict comprehension
+allowed/allowed.py:264: if expression
+allowed/allowed.py:281: f-string
+allowed/allowed.py:304: :=
+allowed/allowed.py:305: int()
+allowed/allowed.py:401: hasattr()
+allowed/allowed.py:420: isinstance()
+allowed/allowed.py:425: type()
+allowed/allowed.py:525: global
+allowed/allowed.py:528: with
+allowed/allowed.py:682: break
+allowed/allowed.py:683: for-else
+allowed/allowed.py:690: raise
+INFO: checked 3 Python files and 1 notebook
+INFO: the 38 Python constructs listed above are not allowed
+WARNING: other occurrences of the listed constructs may exist (don't use option -f)
 WARNING: didn't check method calls (use option -m)
-WARNING: each construct was reported once; other occurrences may exist

tests/foobar-hfm.txt

@@ -1,4 +1,4 @@
-usage: allowed [-h] [-V] [-f] [-m] [-u UNIT] [-c CONFIG]
+usage: allowed [-h] [-V] [-f] [-m] [-u UNIT] [-c CONFIG] [-v]
                file_or_folder [file_or_folder ...]
 
 Check that the code only uses certain constructs. See http://dsa-
@@ -18,3 +18,4 @@ options:
   -c CONFIG, --config CONFIG
                         allow the constructs given in CONFIG (default:
                         m269.json)
+  -v, --verbose         show additional info as files are processed

tests/invalid-py.txt

@@ -1,2 +1,5 @@
+INFO: using configuration /Users/mw4687/GitHub/dsa-ou/allowed/allowed/m269.json
+INFO: checking tests/invalid.py against units 1–3
 tests/invalid.py:2: SYNTAX ERROR: '(' was never closed
-WARNING: didn't check method calls (use option -m)
+INFO: checked 0 Python files and 0 notebooks
+INFO: didn't check 1 Python file or notebook due to syntax or other errors

tests/tests.sh

@@ -21,8 +21,8 @@ elif [ $1 = "run" ]; then
     echo; echo "-c invalid.json"; echo "---"
     $cmd -c tests/invalid.json foobar | diff -w - tests/invalid-c.txt
     # check with invalid Python code
-    echo; echo "invalid.py"; echo "---"
-    $cmd tests/invalid.py | diff -w - tests/invalid-py.txt
+    echo; echo "-v -u 3 invalid.py"; echo "---"
+    $cmd -v -u 3 tests/invalid.py | diff -w - tests/invalid-py.txt
     # check the example code and notebook
     echo; echo "sample.py"; echo "---"
     $cmd tests/sample.py | diff -w - tests/sample-py.txt
@@ -32,20 +32,20 @@ elif [ $1 = "run" ]; then
     $cmd tests/sample.ipynb | diff -w - tests/sample-nb.txt
     echo; echo "sample.ipynb -m"; echo "---"
     $cmd tests/sample.ipynb -m | diff -w - tests/sample-nb-m.txt
-    echo; echo "-f sample.ipynb allowed/"; echo "---"
-    $cmd -f tests/sample.ipynb allowed | diff -w - tests/folder-first.txt
+    echo; echo "-vf sample.ipynb allowed/"; echo "---"
+    $cmd -vf tests/sample.ipynb allowed | diff -w - tests/folder-first.txt
 elif [ $1 = "create" ]; then
     $cmd foobar -fm > tests/foobar-fm.txt
     $cmd foobar -hfm > tests/foobar-hfm.txt
     $cmd tests/invalid.ipynb > tests/invalid-nb.txt
     $cmd -c tests/sample.py foobar > tests/sample-c.txt
     $cmd -c tests/invalid.json foobar > tests/invalid-c.txt
-    $cmd tests/invalid.py > tests/invalid-py.txt
+    $cmd -v -u 3 tests/invalid.py > tests/invalid-py.txt
     $cmd tests/sample.py > tests/sample-py.txt
     $cmd tests/sample.py -m > tests/sample-py-m.txt
     $cmd tests/sample.ipynb > tests/sample-nb.txt
     $cmd tests/sample.ipynb -m > tests/sample-nb-m.txt
-    $cmd -f tests/sample.ipynb allowed > tests/folder-first.txt
+    $cmd -vf tests/sample.ipynb allowed > tests/folder-first.txt
 else
     echo "Usage: ./tests.sh [run|create]"
 fi