Skip to content

Add mypy to CI: fix real type errors and generate typed protobuf stubs #30

Closed

Description

@craigmcchesney

Summary

Make mypy src/ clean enough to enforce as a blocking CI check. Split out of #29, where a baseline scan found that enforcing it then was not viable.

Re-triaged 2026-09-24. The baseline has more than doubled since this was filed (175 → 426 errors) as the client grew, and a third genuine error has appeared. The work is also now split: this ticket covers the near-term CI gate only, and will be implemented alongside #56 as one CI/release-tooling change. The mypy-protobuf half moves to osprey-dcs/dp-grpc#158, because the stubs are generated by dp-grpc's workflow and synced here (e.g. #49) — see "Split out" below.

Current state

mypy src/ reports 426 errors across 27 files (34 checked). Almost none are real defects:

Error code Count What it actually is
name-defined 252 Name "query_pb2.ColumnTable" is not defined and similar
attr-defined 134 Module has no attribute "PvSelector" and similar
import-untyped 35 grpc (22), google.protobuf (10), pandas (2), yaml (1)
valid-type / operator 2 TimestampInput alias — knock-on from the untyped common_pb2.Timestamp it names
assignment / misc 3 genuine — see below

The name-defined / attr-defined errors share one root cause: the generated _pb2.py files in src/dp_python_lib/grpc/ carry no type information, so mypy cannot see any protobuf message class. The runtime is fine — this is purely a type-checking blind spot.

By file, the noise concentrates in the modules that build protobuf messages most heavily: data_frame.py (79), machine_config_client.py (65), annotations_client.py (43), query_client.py (39), data_frame_conversions.py (39), dataset_client.py (37).

The 3 real errors

mldp_client.py:120: error: Incompatible types in assignment (expression has type "None", variable has type "AnnotationClient")  [assignment]
mldp_client.py:130: error: Incompatible types in assignment (expression has type "None", variable has type "QueryClient")  [assignment]
data_frame_conversions.py:209: error: Value expression in dictionary comprehension has incompatible type "dict[str, Any] | None"; expected type "dict[str, Any]"  [misc]
  • mldp_client.py — self.annotation / self.query are a client in one branch and None in the other; mypy infers the non-optional type from the first assignment. The None is intended (CLAUDE.md documents client.query as None without a query channel), so the fix is an explicit AnnotationClient | None / QueryClient | None annotation.
  • data_frame_conversions.py:209 — new since filing. data_frame_image_descriptors() is annotated dict[str, dict[str, Any]], but image_descriptor_dict() can return None. Either the return annotation is wrong or the None case needs handling; decide which rather than just widening the type.

Scope — near-term CI gate

  • Fix the 3 real errors
  • Add typing stubs to the dev extra: types-PyYAML, types-protobuf, and pandas-stubs (or ignore pandas missing imports if the stubs prove too noisy against the lazy-import pattern in the conversion modules)
  • mypy config ([tool.mypy] in pyproject.toml): ignore_missing_imports for grpc (no official stub package), and suppress the generated dp_python_lib.grpc package
  • Confirm the TimestampInput valid-type / operator errors clear once the protobuf module is suppressed; if not, fix the alias
  • Add mypy src/ to the quality job in ci.yml once the above brings it to zero

This is a blunt instrument — suppressing the protobuf module leaves client code that touches protobuf messages effectively unchecked — but it turns mypy from "426 errors, unusable" into a gate that catches regressions in hand-written logic. It has already earned its keep once: the data_frame_conversions.py error above appeared after filing and nothing caught it.

Split out — typed protobuf stubs (dp-grpc)

The real fix is mypy-protobuf emitting .pyi files alongside each _pb2.py. That resolves the ~390 protobuf errors properly and gives genuine checking of the hand-built requests throughout the client — wrong field names and message types caught at check time instead of runtime.

That work belongs in dp-grpc: the Python stubs are generated by dp-grpc's workflow and delivered here by the automated stub-sync PR, so the change is to that generation step. It must also extend the relative-import post-processing (CLAUDE.md, "Import Fix Process") to the .pyi files. Once typed stubs arrive in a sync, the dp_python_lib.grpc suppression added here can be removed. Tracked in osprey-dcs/dp-grpc#158.

Context

Split out of #29 (GHA workflows), which shipped CI with ruff, the unit test suite, and the cookbook snippet checker. mypy stays scoped to the cookbook checker (.dev/tools/check-cookbook-snippets.py) until this lands; it works there today because it type-checks usage of the library rather than its internals.

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions