diff --git a/.github/check_pr.py b/.github/check_pr.py new file mode 100755 index 0000000..52546e0 --- /dev/null +++ b/.github/check_pr.py @@ -0,0 +1,204 @@ +#!/usr/bin/env python3 +"""PR gate: repo structure + the rules in CONTRIBUTING.md. Run from the repo root.""" + +import argparse +import ast +import json +import re +import subprocess +import sys +from pathlib import Path + +# Every entry allowed at the repo root. Adding one is a structural change. +TOP_LEVEL = { + ".github", ".gitignore", "CONTRIBUTING.md", "LICENSE", "README.md", + "requirements.txt", "data", "Decorators & Namespaces", "docs", + "notebooks", "resources", "scripts", "tests", +} +# Which extension belongs in which directory, and how deep a new file may sit. +DIR_RULES = {"scripts": ".py", "notebooks": ".ipynb", "docs": ".md", "tests": ".py"} +FREEFORM = {"data", "resources", "Decorators & Namespaces", ".github"} + +IMPORT_ALIASES = { + "cv2": "opencv-python", "PIL": "pillow", "sklearn": "scikit-learn", + "bs4": "beautifulsoup4", "yaml": "pyyaml", "dotenv": "python-dotenv", +} +LOCAL_PATH = re.compile(r"/Users/|/home/[a-z]|[A-Z]:\\Users") +DESTRUCTIVE = re.compile(r"shutil\.(rmtree|move)|os\.(remove|unlink|rmdir|rename)|\.unlink\(|\.replace\(") + +errors, warnings = [], [] + + +def fail(path, msg): + errors.append(f"{path}: {msg}") + + +def warn(path, msg): + warnings.append(f"{path}: {msg}") + + +def changed_files(base): + out = subprocess.run( + ["git", "diff", "--name-status", "--find-renames", f"{base}...HEAD"], + capture_output=True, text=True, check=True, + ).stdout + changes = [] + for line in out.splitlines(): + parts = line.split("\t") + changes.append((parts[0][0], parts[1:])) + return changes + + +def requirement_names(): + names = set() + for line in Path("requirements.txt").read_text().splitlines(): + line = line.split("#")[0].strip() + if line: + names.add(re.split(r"[<>=!\[;\s]", line)[0].lower().replace("_", "-")) + return names + + +def imported_roots(tree): + roots = set() + for node in ast.walk(tree): + if isinstance(node, ast.Import): + roots.update(a.name.split(".")[0] for a in node.names) + elif isinstance(node, ast.ImportFrom) and not node.level and node.module: + roots.add(node.module.split(".")[0]) + return roots + + +def check_structure(changes): + for status, paths in changes: + target = paths[-1] + top = target.split("/")[0] + if status in "AR" and top not in TOP_LEVEL: + fail(target, f"new top-level entry '{top}'. Put it in one of: {', '.join(sorted(DIR_RULES))}") + continue + if status == "R": + fail(target, f"renamed/moved from '{paths[0]}'. Open an issue first — moves break links in the docs and notebooks") + if status == "D": + fail(target, "deleted. Deletions need a maintainer's sign-off; open an issue instead") + if status == "A" and top in DIR_RULES: + bits = target.split("/") + if len(bits) != 2: + fail(target, f"nested under {top}/. One flat file per contribution, no subdirectories") + elif not target.endswith(DIR_RULES[top]): + fail(target, f"{top}/ holds only {DIR_RULES[top]} files") + + +def test_sources(_cache={}): + # Why: a script counts as checked if a test file names it + if not _cache: + files = list(Path("tests").glob("test_*.py")) + list(Path("scripts").glob("test_*.py")) + _cache["text"] = "\n".join(p.read_text(errors="replace") for p in files) + return _cache["text"] + + +def check_script(path, reqs, repo_modules): + source = Path(path).read_text(encoding="utf-8", errors="replace") + try: + tree = ast.parse(source, filename=path) + except SyntaxError as exc: + fail(path, f"does not parse: line {exc.lineno}: {exc.msg}") + return + + for root in sorted(imported_roots(tree)): + if root in sys.stdlib_module_names or root in repo_modules: + continue + pkg = IMPORT_ALIASES.get(root, root).lower().replace("_", "-") + if pkg not in reqs: + fail(path, f"imports third-party '{root}' — add '{pkg}' to requirements.txt and say why in a comment") + + if DESTRUCTIVE.search(source) and "--dry-run" not in source: + fail(path, "moves or deletes files but has no --dry-run flag") + + has_logic = any(isinstance(n, (ast.For, ast.While, ast.FunctionDef)) for n in ast.walk(tree)) + stem = Path(path).stem + tested = Path(f"tests/test_{stem}.py").exists() or stem in test_sources() + if has_logic and "assert" not in source and not tested: + warn(path, "real logic but no check — add a few asserts under `if __name__ == \"__main__\":`") + + +def check_notebook(path): + try: + nb = json.loads(Path(path).read_text(encoding="utf-8")) + except json.JSONDecodeError as exc: + fail(path, f"is not valid notebook JSON: {exc}") + return + size = 0 + for cell in nb.get("cells", []): + blob = json.dumps(cell.get("outputs", [])) + size += len(blob) + if LOCAL_PATH.search(blob): + fail(path, "output contains a local filesystem path — clear the outputs before committing") + return + if size > 1_000_000: + warn(path, f"{size // 1000} KB of saved output — clear the outputs to keep clones small") + + +def main(): + ap = argparse.ArgumentParser(description=__doc__) + ap.add_argument("--base", default="origin/main", help="branch or ref to diff against") + ap.add_argument("--selftest", action="store_true") + args = ap.parse_args() + + if args.selftest: + selftest() + return 0 + + if not Path("CONTRIBUTING.md").exists(): + print("Run this from the repo root.", file=sys.stderr) + return 2 + + try: + changes = changed_files(args.base) + except subprocess.CalledProcessError: + print(f"Cannot diff against '{args.base}'. Fetch it first: git fetch origin main", file=sys.stderr) + return 2 + + if not changes: + print(f"No changes against {args.base}.") + return 0 + + check_structure(changes) + reqs = requirement_names() + # Why: sibling scripts and the scripts/ and tests/ packages are local, not PyPI + repo_modules = {p.stem for p in Path("scripts").glob("*.py")} | set(DIR_RULES) + for status, paths in changes: + target = paths[-1] + if status == "D" or not Path(target).exists(): + continue + if target.endswith(".py"): + check_script(target, reqs, repo_modules) + elif target.endswith(".ipynb"): + check_notebook(target) + + for line in warnings: + print(f"warning: {line}") + for line in errors: + print(f"error: {line}") + print(f"\n{len(changes)} changed file(s), {len(errors)} error(s), {len(warnings)} warning(s).") + if errors: + print("See CONTRIBUTING.md for the rule behind each error.") + return 1 if errors else 0 + + +def selftest(): + tree = ast.parse("import os, numpy as np\nfrom pathlib import Path\nfrom . import x") + assert imported_roots(tree) == {"os", "numpy", "pathlib"} + assert re.split(r"[<>=!\[;\s]", "pdf2docx>=0.5")[0] == "pdf2docx" + assert LOCAL_PATH.search("/Users/someone/data.csv") + assert not LOCAL_PATH.search("data/sample.txt") + assert DESTRUCTIVE.search("shutil.rmtree(p)") and not DESTRUCTIVE.search("os.listdir(p)") + errors.clear() + check_structure([("A", ["tools/x.py"]), ("A", ["scripts/deep/x.py"]), ("A", ["scripts/a.txt"]), ("D", ["README.md"])]) + assert len(errors) == 4, errors + errors.clear() + check_structure([("A", ["scripts/ok.py"]), ("M", ["README.md"])]) + assert not errors, errors + print("selftest ok") + + +if __name__ == "__main__": + sys.exit(main()) diff --git a/.github/workflows/pr-check.yml b/.github/workflows/pr-check.yml new file mode 100644 index 0000000..f6c6119 --- /dev/null +++ b/.github/workflows/pr-check.yml @@ -0,0 +1,26 @@ +name: PR check + +on: + pull_request: + branches: [main] + +jobs: + check: + runs-on: ubuntu-latest + steps: + - uses: actions/checkout@v4 + with: + fetch-depth: 0 + + - uses: actions/setup-python@v5 + with: + python-version: "3.11" + + - name: Structure and CONTRIBUTING rules + run: python .github/check_pr.py --base origin/${{ github.base_ref }} + + - name: Every script compiles + run: python -m compileall -q scripts + + - name: Tests + run: python -m unittest discover tests -v diff --git a/requirements.txt b/requirements.txt index ec909cf..fc9e5ff 100644 Binary files a/requirements.txt and b/requirements.txt differ diff --git a/tests/test_algorithms.py b/tests/test_algorithms.py new file mode 100644 index 0000000..fe17f94 --- /dev/null +++ b/tests/test_algorithms.py @@ -0,0 +1,101 @@ +"""Tests for the search and sort scripts. + +Run from the repo root: + python -m unittest discover tests +""" + +import contextlib +import importlib.util +import io +import unittest +from pathlib import Path + +SCRIPTS = Path(__file__).resolve().parent.parent / "scripts" + + +def load(filename): + # Why: names like "01.TwoSum.py" can't be imported with `from scripts.x import y` + spec = importlib.util.spec_from_file_location(filename, SCRIPTS / filename) + module = importlib.util.module_from_spec(spec) + with contextlib.redirect_stdout(io.StringIO()): # Why: some scripts print at import + spec.loader.exec_module(module) + return module + + +class TestBinarySearch(unittest.TestCase): + def setUp(self): + self.search = load("BinarySearch.py").binarySearch + + def find(self, array, x): + return self.search(array, x, 0, len(array) - 1) + + def test_finds_element(self): + array = [3, 4, 5, 6, 7, 8, 9] + for i, value in enumerate(array): + self.assertEqual(self.find(array, value), i) + + def test_missing_element(self): + self.assertEqual(self.find([3, 4, 5], 99), -1) + + def test_empty_array(self): + self.assertEqual(self.find([], 1), -1) + + +class TestLinearSearch(unittest.TestCase): + def setUp(self): + self.search = load("LinearSearch.py").linearSearch + + def test_finds_element(self): + array = [2, 4, 0, 1, 9] + self.assertEqual(self.search(array, len(array), 1), 3) + + def test_returns_first_match(self): + array = [7, 7, 7] + self.assertEqual(self.search(array, len(array), 7), 0) + + def test_missing_element(self): + self.assertEqual(self.search([2, 4], 2, 5), -1) + + +class TestMergeSort(unittest.TestCase): + def setUp(self): + self.sort = load("MergeSort.py").mergeSort + + def sorted_copy(self, array): + array = list(array) + self.sort(array) # Why: sorts in place, returns None + return array + + def test_sorts_in_place(self): + self.assertEqual(self.sorted_copy([6, 5, 12, 10, 9, 1]), [1, 5, 6, 9, 10, 12]) + + def test_duplicates_and_negatives(self): + self.assertEqual(self.sorted_copy([3, -1, 3, 0, -1]), [-1, -1, 0, 3, 3]) + + def test_short_inputs(self): + self.assertEqual(self.sorted_copy([]), []) + self.assertEqual(self.sorted_copy([42]), [42]) + self.assertEqual(self.sorted_copy([2, 1]), [1, 2]) + + +class TestTwoSum(unittest.TestCase): + def setUp(self): + self.solve = load("01.TwoSum.py").Solution().twoSum + + def test_returns_indices_of_pair(self): + nums, target = [2, 7, 11, 15], 9 + i, j = self.solve(nums, target) + self.assertNotEqual(i, j) + self.assertEqual(nums[i] + nums[j], target) + + def test_unsorted_input(self): + nums, target = [3, 2, 4], 6 + i, j = self.solve(nums, target) + self.assertEqual(sorted([nums[i], nums[j]]), [2, 4]) + + def test_no_pair_returns_empty_list(self): + self.assertEqual(self.solve([1, 2, 3], 100), []) + + +if __name__ == "__main__": + unittest.main()