Skip to content

refactor: annotate remaining functions and arguments (#66) - #78

Open
tschm wants to merge 1 commit into
alihaskar:masterfrom
tschm:fix/66-annotations
Open

refactor: annotate remaining functions and arguments (#66)#78
tschm wants to merge 1 commit into
alihaskar:masterfrom
tschm:fix/66-annotations

Conversation

@tschm

@tschm tschm commented Jul 28, 2026

Copy link
Copy Markdown
Contributor

Summary

Rhiza v1.2.2 adds flake8-annotations (ANN) to the ruff selection and runs mypy --strict. Both flag the same defect from two angles, which is why this was the highest-leverage item on the scorecard: one pass moves two gates.

Manual annotations

File Change
core/server.py get_response(scope), health_check, not_found_handler, server_error_handler
core/lifecycle.py __enter__ -> "ChartServer", __exit__ + its three params
api/routes.py get_data -> DataResponse
data/ingestion.py `slice_opt(arr) -> list[Any]
demo.py generate_ohlc -> tuple[...]

Plus ruff's own fixes

Ten -> None (and one -> str on root, which returns an HTML string) were applied via ruff check --select ANN --fix --unsafe-fixes — the same flags the pre-commit hook itself uses. Without these the ANN family isn't actually clean, so the issue's acceptance criterion needs them.

Two supporting decisions

  • scope is typed MutableMapping[str, Any] — what starlette.types.Scope aliases to — rather than importing starlette. Starlette is a FastAPI transitive dep, not a declared one, so importing it directly would trip deptry.
  • JSONResponse moved to the module imports so the handler return types can name it. This also removes two function-local re-imports of the same symbol.

The two unused handler args (ARG001) are deliberately left to #74, which owns them.

Verification

$ ruff@0.15.22 check --select ANN src demo.py
All checks passed!                     # v1.2.2 rule family, clean

$ mypy --strict src | grep -cE 'no-untyped-def|no-untyped-call'
0                                      # was 13
Found 43 errors  (was 55; remainder is #67/#68)

$ make fmt        -> all hooks Passed
$ make typecheck  -> ty: All checks passed!
$ make test       -> 154 passed, Total coverage: 100.00%

Note: master is on rhiza v0.18.8, whose ruff selection has no ANN and whose typecheck doesn't run mypy --strict. The v1.2.2 rule set was therefore run explicitly, pinned to the same ruff version the hook uses.

Closes #66

Rhiza v1.2.2 adds flake8-annotations (ANN) to the ruff selection and runs
mypy --strict, both of which flag the same missing annotations.

Manual annotations:
  server.py     get_response(scope), health_check, not_found_handler,
                server_error_handler
  lifecycle.py  __enter__ -> "ChartServer", __exit__ + its three params
  routes.py     get_data -> DataResponse
  ingestion.py  slice_opt(arr) -> list[Any] | None
  demo.py       generate_ohlc -> tuple[...]

Plus ten `-> None`/`-> str` returns applied by ruff's own fix, matching the
pre-commit hook's --unsafe-fixes configuration.

Two supporting changes:
  - `scope` is typed MutableMapping[str, Any] (what starlette.types.Scope
    aliases to) rather than importing starlette, which is not a declared
    dependency and would trip deptry.
  - JSONResponse moves to the module imports so the handler return types can
    reference it, removing two function-local re-imports.

ANN now clean; mypy no-untyped-def/no-untyped-call drop from 13 to 0
(total mypy errors 55 -> 43, remainder tracked by alihaskar#67 and alihaskar#68).

Closes alihaskar#66

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings July 28, 2026 09:11

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR completes a targeted pass of Python type annotations across the codebase to satisfy the newly-enabled Ruff ANN* rules and reduce mypy --strict “untyped def/call” findings, without changing runtime behavior.

Changes:

  • Added missing argument and return type annotations in the FastAPI server (static file handler, endpoints, and exception handlers).
  • Added missing -> None / context-manager method annotations in the server lifecycle controller, including __enter__ / __exit__ typing.
  • Added/updated return types in the API route, data ingestion helper, and demo utilities.

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
src/pycharting/data/ingestion.py Adds explicit return type for DataManager.__init__ and types the slice_opt helper used for optional array slicing.
src/pycharting/core/server.py Types ASGI scope for StaticFiles.get_response, adds endpoint/handler return types, and hoists JSONResponse import for typed handler returns.
src/pycharting/core/lifecycle.py Adds missing -> None annotations on lifecycle helpers and fully types context manager methods (__enter__ / __exit__).
src/pycharting/api/routes.py Annotates get_data to return the DataResponse model.
demo.py Annotates demo helpers (generate_ohlc, run_demo, main) to satisfy annotation checks and clarify returned structures.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Annotate un-annotated functions and arguments across src/ and demo.py

2 participants