Trajectory Metadata & Config Parsing#49
Conversation
|
Also added in:
|
|
There was a problem hiding this comment.
Pull Request Overview
This PR introduces a robust type system for trajectory metadata and improves configuration parsing throughout evalio. The key changes transform the architecture from using untyped dictionaries to strongly-typed metadata classes, along with introducing comprehensive error handling similar to Rust's Result pattern.
Key changes:
- Introduced typed
MetadataandExperimentclasses to replace untyped dictionaries for trajectory metadata - Implemented parser modules for datasets and pipelines with comprehensive error handling
- Added support for window-based RTE computation in meters and seconds (previously fixed timesteps)
Reviewed Changes
Copilot reviewed 42 out of 43 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/test_stats.py | Removed metadata dictionary parameters from Trajectory initialization |
| tests/test_parsing.py | New test file for dataset and pipeline configuration parsing |
| tests/test_io.py | New test file for trajectory serialization/deserialization with metadata |
| tests/test_dataset_impl.py | Updated imports to use new dataset parser module |
| tests/test_csv_loading.py | Updated type annotations to use new GroundTruth metadata class |
| tests/test_cli_ls.py | Removed deprecated dataset building test |
| tests/make_test_data.py | Updated to use new dataset registration API |
| tests/get_lens.py | Updated to use new dataset registration API |
| python/evalio/utils.py | Added CustomException base class and pascal_to_snake utility function |
| python/evalio/types/extended.py | New file with Experiment and ExperimentStatus classes |
| python/evalio/types/base.py | New file with base Trajectory, Metadata, and GroundTruth classes |
| python/evalio/types/init.py | Reorganized type exports with new metadata classes |
| python/evalio/types.py | Removed (replaced by modular types/ directory) |
| python/evalio/stats.py | Updated for new window-based RTE computation and typed trajectories |
| python/evalio/rerun.py | Updated for new metadata types and visualization API |
| python/evalio/pipelines/parser.py | New pipeline configuration parser with error types |
| python/evalio/pipelines/init.py | Added parser exports |
| python/evalio/datasets/parser.py | New dataset configuration parser with error types |
| python/evalio/datasets/newer_college_2020.py | Removed type ignore comment |
| python/evalio/datasets/multi_campus.py | Fixed CSV field name and removed type ignore comment |
| python/evalio/datasets/base.py | Updated to use new metadata system and moved utility functions |
| python/evalio/datasets/init.py | Added parser exports |
| python/evalio/cli/writer.py | Removed (functionality moved to Trajectory.to_file) |
| python/evalio/cli/stats.py | Complete rewrite supporting new metadata and multiple windows |
| python/evalio/cli/run.py | Updated to use new experiment system with metadata |
| python/evalio/cli/parser.py | Removed (replaced by modular parser modules) |
| python/evalio/cli/ls.py | Updated to use new parser modules |
| python/evalio/cli/dataset_manager.py | Updated to use new dataset parser |
| python/evalio/cli/completions.py | Updated to use new parser modules |
| python/evalio/cli/init.py | Added custom module registration callback |
| python/evalio/init.py | Added automatic custom module registration from environment |
| pyproject.toml | Added polars and joblib dependencies |
| mkdocs.yml | Updated documentation settings |
| docs/ref/pipelines.md | Updated to explicitly list pipeline members |
| docs/ref/datasets.md | Simplified to show all dataset members |
| docs/quickstart.md | Updated example code to use new import patterns |
| cpp/evalio/types.h | Added SE3::error and SE3::distance helper methods |
| cpp/bindings/types.h | Added Python bindings for new SE3 methods and copy constructors |
| cpp/bindings/ros_pc2.h | Added parse_csv_line and closest helper functions |
| cpp/bindings/main.cpp | Disabled nanobind leak warnings |
| README.md | Updated with citation information and simplified quick start |
| CITATION.bib | New citation file for arXiv paper |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| # Parse the filtering options | ||
| filter_method: Callable[[dict[str, Any]], bool] | ||
| if filter_str is None: | ||
| filter_method = lambda r: True # noqa: E731 |
There was a problem hiding this comment.
Using lambda with noqa to bypass linting is acceptable here, but the filter_method on line 293 uses eval() which is a security risk. The eval() call allows arbitrary code execution through the filter_str parameter. Consider using a safer alternative like ast.literal_eval() or a custom expression parser that doesn't allow arbitrary code execution.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
This is quite a large PR that started as an effort to tackle the ugly large dictionary we were passing around with Trajectories by adding some types to it. It has expanded to include
I'm going to sit on this PR a bit as I use it for research to make sure it's gets some run before getting merged