Document CVAT cloud storage id kwarg #5456
Conversation
WalkthroughThis change updates the annotation functionality for cloud-backed datasets in CVAT. The Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant SampleCollection
participant CVAT
Client->>SampleCollection: Call annotate(anno_key, label_field, cloud_storage_id, cloud_manifest)
alt cloud_manifest is True
Note over SampleCollection: Use default manifest.jsonl
else Custom URL provided
Note over SampleCollection: Use provided cloud_manifest URL
end
SampleCollection->>CVAT: Forward annotation request
CVAT-->>SampleCollection: Return annotation results
SampleCollection-->>Client: Return final output
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
Documentation and Community
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (3)
docs/source/teams/cloud_media.rst (3)
743-753: Example forcloud_storage_idUsage Is Well-Structured
The example code block demonstrates how to call theannotate()method with the newcloud_storage_id=51argument. The snippet is succinct and correctly indented. Optionally, you might consider adding a brief inline comment to indicate that the value “51” is a placeholder for the actual storage ID.
754-759: Documentation for thecloud_manifestParameter
The explanation for using thecloud_manifestparameter is clear—it instructs users to provide the URL of the manifest file in their cloud bucket when configured. For improved clarity, consider explicitly stating that the URL must be accessible and well-formed.
786-788: Clarification on Cloud Manifest Content Requirements
The note emphasizes that the cloud storage must contain all media files in the sample collection being annotated. This is an important clarification. You might consider rewording for enhanced clarity (for example, "all media files referenced in the sample collection" may read more clearly), but overall the message is sound.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
docs/source/teams/cloud_media.rst(3 hunks)
🔇 Additional comments (4)
docs/source/teams/cloud_media.rst (4)
739-742: Clear Documentation forcloud_storage_idParameter
The updated text explains that thecloud_storage_idparameter must be provided to theannotate()method to specify the integer ID of the configured CVAT cloud storage. This change is clear and consistent with the PR objectives. Please verify that the expected type (an integer) is also reflected in the API signature.
760-770: Cloud Manifest Example Code Block Is Clear and Informative
The example for using thecloud_manifestparameter (with a URL value) is well presented. It follows the established pattern shown in thecloud_storage_idexample and accurately demonstrates how to integrate the new parameter into theannotate()call.
771-774: Default Manifest Handling Explanation Is Concise
The updated text clearly explains that if the cloud manifest has the default name (manifest.jsonl) and is located at the bucket’s root, the user can simply passcloud_manifest=True. This is a concise and user-friendly way to cover the default behavior.
775-783: Example for Defaultcloud_manifestUsage Is Correct
The code block demonstrating the use ofcloud_manifest=Trueis correct and consistent with the accompanying documentation. This example effectively communicates the convenience option for when the default manifest naming is in use.
What changes are proposed in this pull request?
Adds documentation for a new teams-only CVAT argument for interacting with cloud media
(Drafted until teams feature is merged)
How is this patch tested? If it is not, please explain why.
(Details)
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
New Features
Documentation