|
| 1 | +--- |
| 2 | +name: review-python-package |
| 3 | +description: Audit a Python package repository against the DHI Python package standards — layout, pyproject, dependencies, tests, code smells, docs, CI and release setup. Produces a findings report with links to the rule each finding breaks, and offers to open GitHub issues. Use when asked to review, audit or health-check a Python package or repo, or asked "is this a proper package yet". |
| 4 | +--- |
| 5 | + |
| 6 | +# Review a Python package |
| 7 | + |
| 8 | +Audit a whole repository against the DHI Python package standards. This is a repository |
| 9 | +audit — it judges the package as it stands, not a single change. If asked to review a pull |
| 10 | +request or a diff, review only the changed files against the same standards. |
| 11 | + |
| 12 | +## Standards |
| 13 | + |
| 14 | +The rules live in `standards.md` at the root of the `python-package-development` repository, |
| 15 | +published at: |
| 16 | + |
| 17 | +``` |
| 18 | +https://dhi.github.io/python-package-development/standards.html |
| 19 | +``` |
| 20 | + |
| 21 | +Read the local file if this skill is running inside that repository |
| 22 | +(`../../../standards.md` from this skill's directory); otherwise fetch the published page. |
| 23 | +**Read it before reviewing** — do not audit from memory. Every rule carries a severity |
| 24 | +(*Blocker* / *Recommended* / *Nice*) and its heading is the anchor you cite. |
| 25 | + |
| 26 | +## Gather evidence |
| 27 | + |
| 28 | +Work from what the repository actually contains. Do not guess at numbers you can measure. |
| 29 | + |
| 30 | +The reviewer may be on Windows without a POSIX shell, so use the Glob, Read and Grep tools |
| 31 | +for anything that inspects files — never `ls`, `cat`, or a pipe into `grep`. |
| 32 | + |
| 33 | +| What | How | |
| 34 | +| --- | --- | |
| 35 | +| Layout, `.gitignore`, `LICENSE`, `README` | Glob `*` and `.*` at the root | |
| 36 | +| Build system, metadata, dependencies, versioning | Read `pyproject.toml` | |
| 37 | +| CI | Glob `.github/workflows/*`, then Read each | |
| 38 | +| Data committed to git | `git ls-files "*.csv" "*.nc" "*.dfs*" "*.xlsx" "*.zip" "*.parquet"` | |
| 39 | +| Commit hygiene | `git log --oneline -20` | |
| 40 | + |
| 41 | +Then run the linter — ground truth, not a guess. `uvx` so it works without the repository's |
| 42 | +environment installed: |
| 43 | + |
| 44 | +```bash |
| 45 | +uvx ruff check . |
| 46 | +uvx ruff format --check . |
| 47 | +``` |
| 48 | + |
| 49 | +`ruff` reads the repository's `[tool.ruff]` config if there is one. If there is none you are |
| 50 | +seeing the default ruleset, which is not the same as the project's intent — say so in the |
| 51 | +report rather than presenting the output as the project's own standard. |
| 52 | + |
| 53 | +Then read the source: `src/` (or the package directory), `tests/`, `docs/`. |
| 54 | + |
| 55 | +**Do not run** `pytest`, `mypy` or `uv sync` on your own initiative — they are slow, may need |
| 56 | +network or credentials, and an unfamiliar test suite may have side effects. Report what the |
| 57 | +test suite looks like and offer to run it. Whether a type checker runs at all is visible in |
| 58 | +the workflow file — judge that from reading it, not by running one. |
| 59 | + |
| 60 | +## Check for the code smells the course names |
| 61 | + |
| 62 | +Search as a starting point, then read the hits — a match is a candidate, not a finding. |
| 63 | + |
| 64 | +Use the Grep tool over `**/*.py`, showing line numbers. These are regular expressions for |
| 65 | +that tool, not shell commands — do not wrap them in `grep`, and do not escape the `|`. |
| 66 | + |
| 67 | +``` |
| 68 | +def .*=\s*(\[\]|\{\}|set\(\)|dict\(\)|list\(\)) # mutable default arguments |
| 69 | +except\s*: # bare except |
| 70 | +except.*:\s*pass # swallowed error, one-line form |
| 71 | +def [a-z]+[A-Z] # java-like API |
| 72 | +\w+\.\w*\._[a-z] # reaching into another object's internals |
| 73 | +``` |
| 74 | + |
| 75 | +`except.*:\s*pass` only catches the one-line form, so read every `except` block you find. For |
| 76 | +the last pattern, discard the `self._` hits — those are the class's own internals. |
| 77 | + |
| 78 | +The rest have no pattern worth writing; read for them: |
| 79 | + |
| 80 | +| Rule | What to look for | |
| 81 | +| --- | --- | |
| 82 | +| Class variables that should be instance variables | mutable assignment in a class body, outside `__init__` | |
| 83 | +| Modified input arguments | assignment to a parameter's elements inside a function | |
| 84 | +| Mixed return types | multiple `return` statements of different types in one function | |
| 85 | +| Missing docstrings | public functions and classes with no `"""` | |
| 86 | +| Removals with no deprecation path | `git log -p` on the public API vs `CHANGELOG.md` | |
| 87 | + |
| 88 | +## Report |
| 89 | + |
| 90 | +Print the report in the conversation. Do not write a file unless asked. |
| 91 | + |
| 92 | +- One-line verdict first — is this a package someone can install and depend on, or not. |
| 93 | +- Then **Blockers**, **Recommended**, **Nice** in that order. Omit empty sections. |
| 94 | +- Every finding: `file:line` where there is one, what is wrong in one line, the fix, and the |
| 95 | + anchor URL of the rule. `✗` for Blockers, `⚠` for everything else. |
| 96 | +- Say what you checked and found clean — a short "✓ Packaging, docs, CI" line. Silence reads |
| 97 | + as "not checked". |
| 98 | +- Report what you did not check and why (tests not run, package not installed). |
| 99 | + |
| 100 | +``` |
| 101 | +**my_library** — not installable yet |
| 102 | +
|
| 103 | +Blockers |
| 104 | + ✗ pyproject.toml — no [build-system], so pip cannot build this |
| 105 | + https://dhi.github.io/python-package-development/standards.html#pyproject.toml |
| 106 | + ✗ No LICENSE — effectively all rights reserved, colleagues cannot legally use it |
| 107 | + https://dhi.github.io/python-package-development/standards.html#license |
| 108 | + ✗ src/clean.py:22 — mutable default `cart=[]` is shared across every call; use None |
| 109 | + https://dhi.github.io/python-package-development/standards.html#mutable-default-arguments |
| 110 | +
|
| 111 | +Recommended |
| 112 | + ⚠ No .github/workflows/ — tests never run outside your machine |
| 113 | + https://dhi.github.io/python-package-development/standards.html#ci-on-every-push-and-pull-request |
| 114 | +
|
| 115 | +✓ Clean: layout, README, naming, dependency groups |
| 116 | +Not checked: test suite not run (offer: `uv run pytest`) |
| 117 | +``` |
| 118 | + |
| 119 | +Do not pad the report. A package that is in good shape gets a short report saying so. |
| 120 | + |
| 121 | +## Afterwards |
| 122 | + |
| 123 | +Offer, do not act: |
| 124 | + |
| 125 | +- Open a GitHub issue per blocker (`gh issue create`) — **ask first, and confirm the target |
| 126 | + repository**. Creating issues is outward-facing. |
| 127 | +- Run the test suite, or `mypy`. |
| 128 | +- Fix the findings. |
0 commit comments