Modernize repo: src layout, pyproject.toml, CI/docs scaffolding#4
Conversation
|
Caution Review failedThe pull request is closed. ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: ASSERTIVE Plan: Pro Run ID: 📒 Files selected for processing (45)
WalkthroughThis PR modernizes the entire rms-picmaker project infrastructure. It establishes Python best practices via Cursor AI rules, configures the package with setuptools and pyproject.toml, sets up Sphinx documentation with Read the Docs integration, creates GitHub Actions workflows for testing and publishing, provides structured contribution guidelines with issue templates, and includes AI-powered code analysis skills. Minor import fixes align the codebase with the new package structure. ChangesProject Modernization: Tooling, Standards, and Infrastructure Setup
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Tip 💬 Introducing Slack Agent: The best way for teams to turn conversations into code.Slack Agent is built on CodeRabbit's deep understanding of your code, so your team can collaborate across the entire SDLC without losing context.
Built for teams:
One agent for your entire SDLC. Right inside Slack. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Purpose
Bring
rms-picmakeronto the current SETI/RMS Python project template so it can be tested, type-checked, documented, and published like the otherrms-*libraries. Prior to this branch the repository was a flat collection of three legacy scripts (picmaker.py,tiff16.py,colornames.py) at the repo root, with no packaging, no CI, no docs, and no tests directory.Closes #
Changes/Implementation Details
src/picmaker/(picmaker.py,tiff16.py,colornames.py); the package is now installable asrms-picmakerand importable aspicmaker. The original imports insidepicmaker.pywere updated tofrom picmaker.colornames import ColorNames/from picmaker.tiff16 import ....pyproject.toml(PEP 621) with[project]metadata,setuptools_scmfor versioning,[project.scripts] picmaker = "picmaker.picmaker:main"console entry point, anddev/docsoptional dependency groups.requirements.txt: reduced to a single-e .line per the project's dependency-management rule..github/workflows/):run-tests.yml— lint and test job templates (currentlyworkflow_dispatchonly; most steps commented out pending tests).publish_to_pypi.yml/publish_to_test_pypi.yml— release-driven publish workflows.docs/): Sphinx project (conf.py,index.rst,module.rst,contributing.rst), Napoleon enabled, Mermaid + MyST extensions, RTD theme,.readthedocs.yamlfor RTD builds.README.mdwith badges, rewrittenCONTRIBUTING.md, GitHub issue templates and PR template,codecov.yml,.gitignorerefresh,.vscode/settings.json, andscripts/run-all-checks.sh/scripts/read-docs.shdeveloper scripts.CODE_OF_CONDUCT.mdremoved from the repo root; the canonical copy is now linked from the docs site.CODEBASE_REVIEW.mdadded at the repo root — a structured analysis of the legacy source against the new project rules (.cursor/rules/*), enumerating findings by dimension (structure, best-practices, types, testing, CI, packaging, ...) and a recommended priority list. This is intended as a working document for the follow-up cleanup work and is not a user-facing artifact.Type of Change
The "Breaking change" box is checked because the import path moved from top-level
picmaker/tiff16/colornamesmodules at the repo root topicmaker.picmaker/picmaker.tiff16/picmaker.colornames. No prior PyPI release ofrms-picmakerexists, so there are no downstream consumers to break, but the rename is mechanical and worth flagging.Testing
Manual verification:
pip install -e \".[dev]\"succeeds end-to-end in a clean Python 3.12 venv (the previouspyproject.tomlself-reference issue —\"picmaker\"/\"picmaker[docs]\"in thedevextra — was fixed in this branch to\"rms-picmaker[docs]\"so pip no longer tries to resolve a non-existentpicmakerdistribution on PyPI). Nopytestruns yet becausetests/is empty; this is called out as a Critical finding inCODEBASE_REVIEW.md§4.Potential Impacts
import picmaker→from picmaker import picmakerfor the legacy entry points). The console scriptpicmakeris unchanged for end users.workflow_dispatch-triggered, and most steps are commented out. Follow-up PRs should re-enable the matrix and individual checks (this is Priority 1 inCODEBASE_REVIEW.md).picmaker.pywere rewritten to use the new package layout;tiff16.pyandcolornames.pyare byte-identical aside from their new location).Checklist
ruff check,ruff format) — legacy code not yet reformatted; tracked in the review documentmypypasses — legacy code has no annotations; tracked in the review documentNotes
This PR is the packaging/scaffolding half of modernization. It deliberately does not touch the contents of the three legacy source files beyond the import rewrites required by the layout move, so the diff stays reviewable. The work needed to bring the source itself up to the project's
.cursor/rules/python_best_practices.mdcstandard (split the 3,493-linepicmaker.py, add__init__.py/__all__/py.typed, migrateoptparse→argparse, add type annotations, replaceprintwithlogging, fix mutable defaults / builtin shadowing, write tests) is captured inCODEBASE_REVIEW.mdat the repo root and should land in follow-up PRs.The commit history on the branch is rough (
Everything gone,New repo format,Working repo,Prior to reorg); reviewers should look at the squash diff rather than per-commit.Made with Cursor
Summary by CodeRabbit
New Features
Documentation
Chores
pyproject.toml