fix: resolve input types of viewer#187
Conversation
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
Signed-off-by: ktro2828 <kotaro.uetake@tier4.jp>
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
1 similar comment
☂️ Python Coverage
Overall Coverage
New FilesNo new covered files... Modified Files
|
There was a problem hiding this comment.
Pull Request Overview
This pull request enhances type safety and flexibility for the T4 DevKit viewer by updating input types to accept more generic type hints, improving quaternion handling, and adding comprehensive documentation with examples.
- Updated viewer method signatures to accept
Vector3Like,RotationLike, andRoiLiketypes instead of concrete types - Added quaternion conversion helper function and validators for record classes
- Enhanced documentation with detailed rendering examples and clearer installation instructions
Reviewed Changes
Copilot reviewed 5 out of 8 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| t4_devkit/viewer/viewer.py | Updated type hints to generic types, added quaternion conversion helper function |
| t4_devkit/viewer/record/box.py | Added field validators and quaternion conversion for improved type safety |
| docs/tutorials/render.md | Expanded with detailed usage examples and visual outputs for rendering functionality |
| docs/install.md | Clarified installation instructions for users vs developers |
| README.md | Updated installation section headers to distinguish user vs developer instructions |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| import rerun.blueprint as rrb | ||
| from typing_extensions import Self | ||
|
|
||
| from t4_devkit.common.converter import to_quaternion |
There was a problem hiding this comment.
The import of to_quaternion is added but the corresponding removal of unused imports (Quaternion, Roi, Vector3) is missing from line 15. These concrete types should be removed since they're replaced with generic *Like types.
| import rerun as rr | ||
| import rerun.components as rrc | ||
| from attrs import converters, define, field | ||
| from attrs import converters, define, field, validators |
There was a problem hiding this comment.
The import statement is missing the removal of unused imports. Since concrete types are replaced with generic types, imports like Roi and Vector3 from line 12 may no longer be needed if they're only used for type annotations that are now using *Like types.
What
This pull request introduces several improvements to the documentation and codebase for the T4 DevKit viewer, focusing on clearer installation instructions and enhanced type safety for rendering and record classes. The most significant changes are grouped below.
Documentation improvements:
README.mdanddocs/install.mdby distinguishing steps for users ("Install via GitHub") and developers ("Install from source"). [1] [2] [3] [4]docs/tutorials/render.mdwith detailed usage examples for rendering 2D/3D boxes, point clouds, lanelet maps, and images, including new code snippets and visual output.Type safety and code consistency:
t4_devkit/viewer/viewer.pyfor rendering functions to accept more generic types (Vector3Like,RotationLike,RoiLike) instead of concrete types, improving flexibility and consistency. [1] [2] [3] [4] [5] [6] [7] [8] [9]_to_rerun_quaternionto standardize quaternion conversion for rendering transformations.Record class validation:
Recordclasses int4_devkit/viewer/record/box.pywith validators for fields such asrotation,class_id,uuid, andfuture, ensuring type correctness and reducing runtime errors. [1] [2]_append_with_elementsfor 3D box records, supporting more flexible input types.Imports and type hints:
t4_devkit/viewer/record/box.pyandt4_devkit/viewer/viewer.pyfor consistency and to support the new generic types. [1] [2] [3]These changes collectively improve user and developer experience, enforce better type safety, and provide richer documentation for rendering capabilities.