Conversation
|
@DanMcGann if it you floats your boat, would you mind giving a peruse through the new documentation site? Mostly to see if there's anything that you think would be useful for users to be aware of. I know there's a lot of code changes, so don't worry about going through all of those. |
There was a problem hiding this comment.
Pull Request Overview
This PR introduces the new documentation site on GitHub Pages and refines both the public API documentation and C++ bindings. It includes new and updated documentation pages (pipelines, datasets, quickstart, examples), enhanced C++ binding docstrings with improved descriptions, and updated CI/release workflows with cache management for vcpkg.
Reviewed Changes
Copilot reviewed 50 out of 50 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| docs/ref/pipelines.md | Adds pipelines reference documentation. |
| docs/ref/datasets.md | Introduces datasets reference documentation (note potential link text issue). |
| docs/ref/cli.py | Generates CLI documentation using mkdocs-gen-files. |
| docs/quickstart.md | Provides a comprehensive quickstart guide. |
| docs/javascripts/tablesort.js | Adds table sorting support for documentation. |
| docs/javascripts/katex.js | Adds KaTeX support for rendering math expressions. |
| docs/install.md | Updates installation and build-from-source instructions. |
| docs/index.md | Home page for evalio docs with overview bullet points. |
| docs/included.py | Generates tables for included datasets and pipelines. |
| docs/examples/*.md | Provides various examples (pipeline, odometry, evaluation, dataset, data loading). |
| docs/css/tweaks.css | Applies CSS tweaks for better documentation layout. |
| cpp/bindings/types.h | Enhances docstrings and descriptions for core C++ type bindings. |
| cpp/bindings/pipelines/bindings.h | Adds descriptive documentation for pipeline binding classes. |
| cpp/bindings/pipeline.h | Updates the base Pipeline binding with improved docstrings. |
| .github/workflows/release.yml | Updates caching and environment management in release workflow. |
| .github/workflows/ci.yml | Refines CI workflow and docs deployment triggers. |
Comments suppressed due to low confidence (1)
cpp/bindings/pipeline.h:79
- The term 'piplines' appears to be a typo. It should be 'pipelines'.
.doc() = "Base class for all pipelines. This class defines the interface for interacting with piplines, and is intended to be subclassed by specific implementations.";
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
DanMcGann
left a comment
There was a problem hiding this comment.
Okay finally had some time to peruse this!
Overall I think the docs look great!
I have a few notes / suggestions, but most are probably dealt with in separate PRs to expand the Docs.
Small Things
-
See inline review comments
-
CLI Links In the quick start page, maybe link to the CLI info page. Currently it is referenced a few times, but the docs may not be noticed by a reader as they are in the API reference section.
-
Visualization - In the quick start guide I would include more info about visualization. For a user unfamiliar with re-run they may not know to startup the rerun server before
evalio run. Additionally, it could be good to describe the details of what is visualized for each level of verbosity. The names that appear in rerun are "okay" but would be improved if more clearly documented in the docs.
Larger Long-term Additions
-
Metrics - Would be good to have mathematical documentation for how all of the metrics are computed.
-
Pictures The documentation page (primary the About Page) may be the first landing of a potential user. May be good to improve "marketing" on this page with a summary / abstract image.
-
evalio-example A quick look makes me think this could be slightly out of date? Though this is only a guess based on the fact the example repo still uses Pybind11
-
Frames Understanding what each common frame (imu, lidar, GT) is useful for developers (probably not all users). Mainly the "GT" frame is a little confusing. Maybe good to have this documented / depicted somewhere.
|
That's perfect Dan, thanks for taking a look through. I ended up handling all of the smaller suggestions, and will likely open some PRs for the bigger ones. (also I have a branch in evalio-example ready to merge, waiting to do another release of evalio to pypi so it can use that package directly). |
The main contribution of this PR was making a documentation site on github pages.
While filling out the documentation, I hit a few pieces of the API that needed polishing. They include,
evalio.cli.statsinto the newevalio.statsto make it public facing,quick_lenoption to datasets to hardcode lengths instead of having to load data to look them up. This now allows dataset lengths to be shown in ls.evalio runresults directly to a single file in the case of running a single experiment,load_pose_csvtoTrajectory.from_csv. Same change for loading TUM and experiment results as well.Wow that was more than I remembered. Need to be better at splitting up PRs!