Adopt the ruff/pyright/pytest toolchain and gate CI - #44
Merged
Conversation
Add the Makefile, the ruff/pyright/pytest/coverage config, and the markdownlint config; bring the dev extra up to the tools they run; and ignore the junit.xml that the pytest config emits.
Bring the tree to a clean gate (make test: ruff, ruff format, markdownlint, pyright, pytest) without changing valid-input behavior: - Format to line-length 88 and sort imports; convert package-relative imports to absolute (TID252, ban-relative-imports = all). Symbol imports are kept (not module-qualified). - Rewrite docstrings to Google style: Args/Returns/Raises on functions with parameters, attribute-style one-liners on properties; convert the old reStructuredText :param blocks. - Add type annotations; refine loose return types (e.g. str | None, datetime | None), fix UserDict[str, any] (the builtin) to typing.Any, and correct GraphqlError.error_code to str | None. - Remove asserts from source (they are reserved for tests): precondition asserts become explicit raises, and the two assert-await-refresh cases become plain await calls so the request is not tied to an assertion. - Keep the N818 noqa on CompositeException (public name, no rename). - Ship the PEP 561 py.typed marker in the wheel (package-data) so downstream consumers can use the annotations. - Reflow README prose to satisfy markdownlint. make test passes on 3.10.20.
reanalyze_postcard and sighting_from_postcard branched on str / FeedNode with no else, leaving postcard_id unbound (UnboundLocalError) for any other input type. Add an else that raises a clear TypeError. Valid str / FeedNode inputs behave identically. Add a guard test.
Give the README a clear structure and fix its broken examples: - Add the project description and status shields (PyPI version, build, maintenance, GitHub release, license). - Split the content into Installation, Usage, Translations, Development and Releasing sections. - Fix the Translations example: import BirdBuddy from birdbuddy.client (the package root only exports LOGGER/VERBOSE) and instantiate it with credentials instead of referencing the bare class. - Document the development workflow: pyenv, make deps and the make targets (each self-activates the venv), plus pip install -e '.[dev]'. - Document the PyPI release process: bump the version, then make build (python -m build -> sdist + wheel) and make publish (twine check + upload), including how twine reads its credentials.
Run the full gate in CI instead of pytest alone: set up Python and Node (markdownlint and pyright are Node-based), install the toolchain with make deps, then make test (ruff, ruff format --check, markdownlint, pyright, pytest). Upload the junit and coverage reports as artifacts and publish the pytest results as a PR check with dorny/test-reporter. Extend the matrix to Python 3.13 and 3.14.
There was a problem hiding this comment.
Pull request overview
This PR modernizes the repo’s Python toolchain and CI gate by adopting ruff + pyright + pytest/coverage, adding Makefile-driven workflows, and applying formatting/type/docstring updates across the codebase (including a PEP 561 py.typed marker).
Changes:
- Add/standardize the ruff/pyright/pytest/coverage/markdownlint configuration and a Makefile-driven
make deps/make testworkflow. - Gate CI with the Makefile across a broader Python matrix and publish JUnit/coverage artifacts.
- Apply type-annotation/docstring/formatting updates and add a regression test for invalid postcard input types.
Reviewed changes
Copilot reviewed 27 out of 29 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
tests/test_sighting_create.py |
Adds docstrings and a new test covering invalid reanalyze_postcard input types. |
tests/test_issue_40.py |
Adds docstrings and minor formatting tweaks for strategy regression tests. |
tests/test_exceptions.py |
Tightens imports and clarifies regression test docstring wording. |
tests/conftest.py |
Adds fixture docstrings, typing, and refactors fixture construction/patching. |
tests/__init__.py |
Minor docstring punctuation update. |
README.md |
Restructures README; documents installation/dev/release workflow and shields. |
pyrightconfig.json |
Adds pyright configuration (tests execution environment, excludes). |
pyproject.toml |
Updates dev deps; adds ruff/pytest/coverage configuration; ships py.typed. |
Makefile |
Introduces Makefile targets for deps/format/test/schema/build/publish/clean. |
dump_schema.py |
Adds module docstring; uses pathlib for writing schema.json. |
birdbuddy/user.py |
Docstring and minor style updates. |
birdbuddy/sightings.py |
Broad typing/docstring/format updates across sightings models. |
birdbuddy/queries/me.py |
Docstring punctuation update. |
birdbuddy/queries/feeder.py |
Docstring punctuation update. |
birdbuddy/queries/debug.py |
Adds module docstring for schema introspection query. |
birdbuddy/queries/birds.py |
Docstring punctuation update. |
birdbuddy/queries/auth.py |
Docstring punctuation update. |
birdbuddy/queries/__init__.py |
Switches to absolute imports and defines __all__. |
birdbuddy/py.typed |
Adds PEP 561 marker to publish type information. |
birdbuddy/media.py |
Adds typing/docstrings and adjusts media-expiry helper implementation. |
birdbuddy/feeder.py |
Adds typing/docstrings and converts to absolute imports. |
birdbuddy/feed.py |
Adds typing/docstrings and changes edges/nodes to iterators. |
birdbuddy/exceptions.py |
Expands docstrings and clarifies public API compatibility notes. |
birdbuddy/client.py |
Adds typing/docstrings, improves input validation, and updates request handling. |
birdbuddy/birds.py |
Docstring punctuation update. |
birdbuddy/__init__.py |
Docstring punctuation update. |
.markdownlint.yaml |
Adds markdownlint configuration. |
.gitignore |
Ignores junit.xml. |
.github/workflows/python-package.yml |
Gates CI via make deps + make test, expands Python matrix, uploads/publishes results. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Owner
|
Agreed: copilot flagged existing issues, not introduced in this PR 👍 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Split of #40, part 2 of 3. Per your review there, separating pure repo/tooling from linting/functional and test.
Commits
The diff is on the large side because of the ruff formatting and type fixes, but this PR is scoped better than #40 was 😄. You might want to look at 50420dc, 795551c, bb180de, and 0732ac6 separately first (that's the actual work, formatting was either automatically fixed by ruff, or hand-fixed in the case of lines that exceeded Black's 88 chars)
Makefile, theruff/pyright/pytest/coverageconfig, and themarkdownlintconfig. Brings thedevextras inpyproject.tomlup to the latest versions, resolving the failures in Bump the pip group with 5 updates #43. Nowmake testruns the full format/lint/type check/test workflow.ruffandpyrightfindings; adopt Google-style docstrings — set max line length is to 88 (the ruff/Black default), switch to absolute imports, use Google-style docstrings, improve type annotations, and rewriteasserts in non-test code as explicitraises. Ships a PEP 561py.typedmarker so downstream consumers can use our types.reanalyze_postcard/sighting_from_postcardbranched onstr/FeedNode, leavingpostcard_idunbound (UnboundLocalError) for any other input; adds anelsethat raises a clearTypeError, plus a test.make deps+make test. Uploads coverage reports as artifacts for individual CI runs. Extends the matrix to include Python 3.13 and 3.14.Testing
Ran
make teston Python 3.10.20 (lowest supported runtime) —ruff,ruff format --check,markdownlint(0 issues),pyright(0 errors, 0 warnings),pytest(12 passed). Also ran tests locally against 3.12 and 3.14, but of course CI is the final arbiter for the matrix.