Skip to content

Introduce mypy static type-checking#65

Merged
riita10069 merged 5 commits into
autowarefoundation:mainfrom
apgeorg:ci/static-type-checking
Jun 17, 2026
Merged

Introduce mypy static type-checking#65
riita10069 merged 5 commits into
autowarefoundation:mainfrom
apgeorg:ci/static-type-checking

Conversation

@apgeorg

@apgeorg apgeorg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor

Adds mypy static type-checking to the project and CI.

  • New typecheck make target (cd Model && mypy .), folded into make ci.
  • Runs in CI between lint and tests.
  • Fixes the errors mypy surfaced

Config

Lenient by design so typing can be adopted incrementally:

  • ignore_missing_imports — uninstalled optional deps (osmnx, PIL, …) don't fail the build.
  • follow_imports = silent — untyped legacy code doesn't block checking.

So mypy only flags real type errors today; stricter per-module rules can be added later.

apgeorg added 3 commits June 16, 2026 14:59
Signed-off-by: Apostolos Georgiadis <apostolos.georgiadis@nfiniity.com>
Signed-off-by: Apostolos Georgiadis <apostolos.georgiadis@nfiniity.com>
Signed-off-by: Apostolos Georgiadis <apostolos.georgiadis@nfiniity.com>
@riita10069

Copy link
Copy Markdown
Collaborator

One thing I would adjust before merging: the camera_timestamps change seems to weaken an existing invariant. NvidiaAVDataset preloads timestamps for all cameras, but __getitem__ now builds the dict with .get(), and load_camera_frame() accepts np.ndarray | None and falls back to reading the parquet again when a value is missing.

That makes cache misses look like valid fallback behavior, whereas they probably indicate an internal inconsistency. It also risks silently reintroducing per-sample parquet reads.

I’d prefer to keep the stricter invariant:

camera_timestamps = {
    cam_name: self._camera_timestamps[(clip_uuid, cam_name)]
    for cam_name in self.camera_names
}

and keep load_camera_frame(..., camera_timestamps: dict[str, np.ndarray] | None = None) with direct indexing when the cache dict is provided.

@apgeorg

apgeorg commented Jun 16, 2026

Copy link
Copy Markdown
Contributor Author

One thing I would adjust before merging: the camera_timestamps change seems to weaken an existing invariant. NvidiaAVDataset preloads timestamps for all cameras, but __getitem__ now builds the dict with .get(), and load_camera_frame() accepts np.ndarray | None and falls back to reading the parquet again when a value is missing.

That makes cache misses look like valid fallback behavior, whereas they probably indicate an internal inconsistency. It also risks silently reintroducing per-sample parquet reads.

I’d prefer to keep the stricter invariant:

camera_timestamps = {
    cam_name: self._camera_timestamps[(clip_uuid, cam_name)]
    for cam_name in self.camera_names
}

and keep load_camera_frame(..., camera_timestamps: dict[str, np.ndarray] | None = None) with direct indexing when the cache dict is provided.

thank you @riita10069 for your clarification.. reverted!

@riita10069 riita10069 merged commit 9e1d0af into autowarefoundation:main Jun 17, 2026
2 checks passed

@riita10069 riita10069 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

Thanks

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.

2 participants