ls upgrade#28
Merged
Merged
Conversation
Contributor
There was a problem hiding this comment.
Pull Request Overview
This pull request upgrades the ls command by adding additional dataset options (pipeline version, sensor brand/model, and disk size) and updating dataset-specific metadata methods. Key changes include removal of redundant static URL methods in loading params, addition of detailed dataset info methods (environment, vehicle), and enhancements to CLI table output with new columns for sensor info and disk size.
Reviewed Changes
Copilot reviewed 21 out of 22 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| python/evalio/datasets/*.py | Removed earlier url() implementations and added dataset info |
| python/evalio/cli/ls.py | Expanded output table with new columns and added unique() helper |
| python/evalio/cli/dataset_manager.py | Updated type import to use standard typing.Annotated |
| python/evalio/cli/writer.py & run.py | Updated tqdm import to use tqdm.rich |
| cpp/evalio/* and cpp/bindings/* | Added brand/model fields and a version() method in pipelines |
Files not reviewed (1)
- cpp/bindings/CMakeLists.txt: Language not supported
Comments suppressed due to low confidence (2)
python/evalio/cli/ls.py:98
- [nitpick] The '--links' option is used in an inverted way (i.e., when false, the URL is wrapped as a hyperlink). Consider renaming the option (e.g., to 'use_hyperlink') or adding a clarifying comment to make its behavior more intuitive.
links_str = d.url()
python/evalio/cli/dataset_manager.py:3
- [nitpick] Switching from 'typing_extensions.Annotated' to 'typing.Annotated' may affect compatibility with earlier Python versions. Ensure that the project’s minimum Python version supports this change, or consider a fallback to maintain compatibility.
from typing import Annotated
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Add a number of additional options to the ls command - version for pipeline, models and disk size for datasets.