FOEPD-2856: Add docs for new camera model / intrinsics / extrinsics#6711
FOEPD-2856: Add docs for new camera model / intrinsics / extrinsics#6711sashankaryal merged 1 commit intodevelopfrom
Conversation
sashankaryal
changed the title
WalkthroughThis pull request adds documentation for camera calibration across two files. One file adds substitution definitions for camera-related constructs (CameraIntrinsics, PinholeCameraIntrinsics, OpenCVCameraIntrinsics, OpenCVFisheyeCameraIntrinsics, CameraIntrinsicsRef, SensorExtrinsics, CameraExtrinsics, CameraProjector). The other expands the user guide with dataset-level storage and resolution of camera intrinsics and sensor extrinsics, matrix constructors and transforms, resolution order, and a 3D projection workflow using CameraProjector. No code, API, or runtime changes are included. Estimated code review effort🎯 2 (Simple) | ⏱️ ~12 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Fix all issues with AI Agents 🤖
In @docs/source/user_guide/using_datasets.rst:
- Around line 5826-5875: The example uses undefined variables camera_to_ego and
ego_to_world when calling compose; update the snippet to either (A) define those
transforms using SensorExtrinsics (e.g., create camera_to_ego and ego_to_world
with SensorExtrinsics or SensorExtrinsics.from_matrix) before calling
camera_to_world = camera_to_ego.compose(ego_to_world), or (B) reuse the existing
extrinsics variable (e.g., camera_to_world = extrinsics.compose(inv_extrinsics)
or similar) so the call to SensorExtrinsics.compose references defined
SensorExtrinsics instances and the example runs correctly.
♻️ Duplicate comments (1)
docs/source/user_guide/using_datasets.rst (1)
5763-6010: Intrinsics/extrinsics, resolution, and CameraProjector docs are coherentThe new sections on camera intrinsics, sensor extrinsics, calibration resolution order, and
CameraProjectorprovide a consistent workflow: dataset-level storage, sample-level overrides, frame naming, and projection/unprojection usage. Terminology (frames, quaternions, 3×3/4×4 matrices,chain_via, groupedgroup_slicebehavior) is coherent and matches the earlier “Storing camera calibration” section.Beyond the minor composition-example fix above, the content is solid.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/source/_includes/substitutions.rstdocs/source/user_guide/using_datasets.rst
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T17:24:11.588Z
Learnt from: brimoor
Repo: voxel51/fiftyone PR: 6373
File: docs/source/dataset_zoo/datasets/ucf101.rst:32-0
Timestamp: 2025-10-16T17:24:11.588Z
Learning: In FiftyOne documentation, dataset zoo classes follow the pattern `fiftyone.zoo.datasets.base.{DatasetName}Dataset` and should be referenced using `:class:` directives (e.g., `:class:`UCF101Dataset <fiftyone.zoo.datasets.base.UCF101Dataset>``). This is a valid and established documentation pattern.
Applied to files:
docs/source/user_guide/using_datasets.rst
📚 Learning: 2025-11-25T15:39:52.528Z
Learnt from: paularamo
Repo: voxel51/fiftyone PR: 6619
File: docs/source/tutorials/cosmos-transfer-integration.ipynb:812-813
Timestamp: 2025-11-25T15:39:52.528Z
Learning: In the cosmos-transfer-integration.ipynb tutorial for FiftyOne grouped datasets: when computing embeddings for original images and Cosmos-Transfer generated outputs, use separate embedding fields (e.g., "image_embeddings" for original images and "output_last_embeddings" for generated outputs) to enable independent similarity analysis and comparison between the two slice types.
Applied to files:
docs/source/user_guide/using_datasets.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
docs/source/_includes/substitutions.rst (1)
72-79: Camera-related substitutions look correct and consistentThe new substitutions follow the existing pattern and point to coherent
fiftyone.core.camera.*targets. No changes requested.docs/source/user_guide/using_datasets.rst (1)
792-873: Storing camera calibration section is clear and aligned with Dataset APIThe new
camera_intrinsics/sensor_extrinsicsdocs and examples are consistent with the rest of the Dataset API documentation (naming, grouped dataset usage,dataset.save()notes, and key format"source::target"). No changes requested.
sashankaryal
changed the title
sashankaryal
force-pushed
the
feat/camera-params-rest
branch
from
d5b375d to
267a96a
Compare
sashankaryal
force-pushed
the
docs/camera
branch
from
954ac04 to
da013ba
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI Agents
In @docs/source/user_guide/using_datasets.rst:
- Around line 5826-5853: The documentation example uses undefined variables
camera_to_ego and ego_to_world when calling SensorExtrinsics.compose, which will
break; fix it by explicitly creating SensorExtrinsics instances (e.g.,
camera_to_ego = SensorExtrinsics(...), ego_to_world = SensorExtrinsics(...))
with appropriate translation/quaternion/source_frame/target_frame values and
then call camera_to_world = camera_to_ego.compose(ego_to_world); ensure variable
names match the ones used later (SensorExtrinsics, compose, camera_to_ego,
ego_to_world, camera_to_world).
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/source/_includes/substitutions.rstdocs/source/user_guide/using_datasets.rst
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T17:24:11.588Z
Learnt from: brimoor
Repo: voxel51/fiftyone PR: 6373
File: docs/source/dataset_zoo/datasets/ucf101.rst:32-0
Timestamp: 2025-10-16T17:24:11.588Z
Learning: In FiftyOne documentation, dataset zoo classes follow the pattern `fiftyone.zoo.datasets.base.{DatasetName}Dataset` and should be referenced using `:class:` directives (e.g., `:class:`UCF101Dataset <fiftyone.zoo.datasets.base.UCF101Dataset>``). This is a valid and established documentation pattern.
Applied to files:
docs/source/user_guide/using_datasets.rst
📚 Learning: 2025-11-25T15:39:52.528Z
Learnt from: paularamo
Repo: voxel51/fiftyone PR: 6619
File: docs/source/tutorials/cosmos-transfer-integration.ipynb:812-813
Timestamp: 2025-11-25T15:39:52.528Z
Learning: In the cosmos-transfer-integration.ipynb tutorial for FiftyOne grouped datasets: when computing embeddings for original images and Cosmos-Transfer generated outputs, use separate embedding fields (e.g., "image_embeddings" for original images and "output_last_embeddings" for generated outputs) to enable independent similarity analysis and comparison between the two slice types.
Applied to files:
docs/source/user_guide/using_datasets.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (2)
docs/source/_includes/substitutions.rst (1)
72-79: Camera substitutions look consistent with existing patternThe new
|CameraIntrinsics|/|SensorExtrinsics|/|CameraProjector|substitutions follow the same Sphinx pattern as the rest of the file and read cleanly. Just ensure the referenced classes infiftyone.core.cameramatch these names so intersphinx links resolve in the built docs.docs/source/user_guide/using_datasets.rst (1)
792-873: Dataset-level calibration section is clear and consistentThe new
camera_intrinsics/sensor_extrinsicsexamples match existing patterns in this guide (dataset-level properties, grouped datasets, explicitdataset.save()), and the key-format explanation ("source_frame::target_frame"/"source_frame") is clear. No changes needed.
sashankaryal
force-pushed
the
feat/camera-params-rest
branch
from
c798919 to
640a228
Compare
sashankaryal
force-pushed
the
feat/camera-params-rest
branch
from
640a228 to
02af9fb
Compare
sashankaryal
force-pushed
the
docs/camera
branch
from
da013ba to
268ffa7
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @docs/source/user_guide/using_datasets.rst:
- Around line 5823-5875: The example uses undefined names `camera_to_ego` and
`ego_to_world`; rename the existing `extrinsics` variable to `camera_to_ego` and
add a second `SensorExtrinsics` instance `ego_to_world` (e.g., a transform from
"ego" to "world") before calling `camera_to_world =
camera_to_ego.compose(ego_to_world)` so the `compose` call uses two defined
SensorExtrinsics objects (`camera_to_ego`, `ego_to_world`) and produces
`camera_to_world`.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/source/_includes/substitutions.rstdocs/source/user_guide/using_datasets.rst
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T17:24:11.588Z
Learnt from: brimoor
Repo: voxel51/fiftyone PR: 6373
File: docs/source/dataset_zoo/datasets/ucf101.rst:32-0
Timestamp: 2025-10-16T17:24:11.588Z
Learning: In FiftyOne documentation, dataset zoo classes follow the pattern `fiftyone.zoo.datasets.base.{DatasetName}Dataset` and should be referenced using `:class:` directives (e.g., `:class:`UCF101Dataset <fiftyone.zoo.datasets.base.UCF101Dataset>``). This is a valid and established documentation pattern.
Applied to files:
docs/source/user_guide/using_datasets.rst
📚 Learning: 2025-11-25T15:39:52.528Z
Learnt from: paularamo
Repo: voxel51/fiftyone PR: 6619
File: docs/source/tutorials/cosmos-transfer-integration.ipynb:812-813
Timestamp: 2025-11-25T15:39:52.528Z
Learning: In the cosmos-transfer-integration.ipynb tutorial for FiftyOne grouped datasets: when computing embeddings for original images and Cosmos-Transfer generated outputs, use separate embedding fields (e.g., "image_embeddings" for original images and "output_last_embeddings" for generated outputs) to enable independent similarity analysis and comparison between the two slice types.
Applied to files:
docs/source/user_guide/using_datasets.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (4)
docs/source/_includes/substitutions.rst (1)
72-79: Camera substitutions look consistent and correctThe new
|CameraIntrinsics|,|PinholeCameraIntrinsics|,|OpenCVCameraIntrinsics|,|OpenCVFisheyeCameraIntrinsics|,|CameraIntrinsicsRef|,|SensorExtrinsics|,|CameraExtrinsics|, and|CameraProjector|substitutions follow the existing pattern and point to plausible module paths.docs/source/user_guide/using_datasets.rst (3)
792-873: Dataset-level camera calibration examples are clear and aligned with APIsThe new “Storing camera calibration” section correctly introduces
camera_intrinsicsandsensor_extrinsicsas dataset-level dictionaries, uses realistic keys (e.g.,"left","left::ego"), and showsdataset.save()after edits, consistent with earlier sections on dataset metadata.
5791-5819: Camera intrinsics usage and matrix constructor look soundThe intrinsics section clearly differentiates the intrinsics models and shows both direct construction (
OpenCVCameraIntrinsics(...)) andPinholeCameraIntrinsics.from_matrix(K). The example importsnumpybefore usingnp.arrayand demonstrates accessingintrinsic_matrix, which matches the intended API.
5878-6009: Calibration resolution andCameraProjectorworkflow read correctlyThe sections on resolving calibration (
resolve_intrinsics,resolve_extrinsics) and onCameraProjectordescribe a coherent resolution order (sample-level intrinsics, refs, then dataset-level intrinsics keyed by slice) and a realistic AV-style workflow (dataset-level static calibration + sample-level ego pose, then projection/unprojection with depth). The examples are consistent with the earlier dataset-level calibration section.Please verify against the current
Dataset.resolve_intrinsics,Dataset.resolve_extrinsics, andCameraProjectorsignatures in code to ensure parameter names and default behaviors (e.g.,chain_viahandling and how sample-level extrinsics likeego_poseare discovered) exactly match the implementation.
sashankaryal
force-pushed
the
feat/camera-params-rest
branch
from
02af9fb to
0843f11
Compare
| | | ||
| """ | ||
|
|
||
| import logging |
sashankaryal
force-pushed
the
docs/camera
branch
from
268ffa7 to
621d590
Compare
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @docs/source/user_guide/using_datasets.rst:
- Around line 792-873: The example for sensor_extrinsics uses dataset without
defining it, causing a NameError; make the snippet self-contained by adding the
necessary import and dataset initialization before assigning
dataset.sensor_extrinsics (e.g., import fiftyone as fo and create or load a
dataset via fo.Dataset() or foz.load_zoo_dataset), then set the
sensor_extrinsics dict with fo.SensorExtrinsics entries and call dataset.save();
keep the existing keys ("left::ego", etc.) and the use of fo.SensorExtrinsics,
camera_intrinsics, and dataset.save() intact.
- Around line 5763-6010: The example uses undefined variables camera_to_ego and
ego_to_world when composing extrinsics; replace or define them: set
camera_to_ego = extrinsics (the already-created fo.SensorExtrinsics) and create
or reference an ego_to_world SensorExtrinsics (e.g., a separate
fo.SensorExtrinsics instance or one resolved from sample/dataset) before calling
compose; then call camera_to_ego.compose(ego_to_world) (or
extrinsics.compose(ego_to_world)) so the composition uses defined
SensorExtrinsics objects.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: ASSERTIVE
Plan: Pro
📒 Files selected for processing (2)
docs/source/_includes/substitutions.rstdocs/source/user_guide/using_datasets.rst
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-10-16T17:24:11.588Z
Learnt from: brimoor
Repo: voxel51/fiftyone PR: 6373
File: docs/source/dataset_zoo/datasets/ucf101.rst:32-0
Timestamp: 2025-10-16T17:24:11.588Z
Learning: In FiftyOne documentation, dataset zoo classes follow the pattern `fiftyone.zoo.datasets.base.{DatasetName}Dataset` and should be referenced using `:class:` directives (e.g., `:class:`UCF101Dataset <fiftyone.zoo.datasets.base.UCF101Dataset>``). This is a valid and established documentation pattern.
Applied to files:
docs/source/user_guide/using_datasets.rst
📚 Learning: 2025-11-25T15:39:52.528Z
Learnt from: paularamo
Repo: voxel51/fiftyone PR: 6619
File: docs/source/tutorials/cosmos-transfer-integration.ipynb:812-813
Timestamp: 2025-11-25T15:39:52.528Z
Learning: In the cosmos-transfer-integration.ipynb tutorial for FiftyOne grouped datasets: when computing embeddings for original images and Cosmos-Transfer generated outputs, use separate embedding fields (e.g., "image_embeddings" for original images and "output_last_embeddings" for generated outputs) to enable independent similarity analysis and comparison between the two slice types.
Applied to files:
docs/source/user_guide/using_datasets.rst
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: build
🔇 Additional comments (1)
docs/source/_includes/substitutions.rst (1)
72-80: Verify all newfiftyone.core.camera.*substitutions resolve to real symbols (doc build blocker if not).If any class name/module path differs from the implementation, Sphinx will emit warnings/errors and/or generate broken links. Ensure these eight classes exist in
fiftyone.core.camera:
CameraIntrinsicsPinholeCameraIntrinsicsOpenCVCameraIntrinsicsOpenCVFisheyeCameraIntrinsicsCameraIntrinsicsRefSensorExtrinsicsCameraExtrinsicsCameraProjector
2 participants
What changes are proposed in this pull request?
Documentation for intrinsics / extrinsics. Main PR is #6700
How is this patch tested? If it is not, please explain why.
Local docs build
Release Notes
Is this a user-facing change that should be mentioned in the release notes?
notes for FiftyOne users.
(Details in 1-2 sentences. You can just refer to another PR with a description
if this PR is part of a larger change.)
What areas of FiftyOne does this PR affect?
fiftyonePython library changesSummary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.