-
Notifications
You must be signed in to change notification settings - Fork 557
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Clarifying model vs source metadata for remote zoo models #4932
Conversation
WalkthroughThe documentation file Changes
Possibly related PRs
Suggested reviewers
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
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (3)
docs/source/model_zoo/remote.rst (3)
278-290
: Great addition of optional metadata for remote sources!The inclusion of optional metadata fields for the remote source itself ('name' and 'url') enhances the flexibility and informativeness of the manifest file. This addition is well-documented with a clear table explaining the new fields.
Consider adding a brief explanation of how these optional fields might be used or displayed within the FiftyOne ecosystem to provide more context for users.
Line range hint
292-332
: Excellent example of a model manifest file!The provided example is comprehensive and clearly illustrates the structure of a manifest file, including both the new remote source metadata and a detailed model declaration. This serves as a valuable reference for users creating their own manifest files.
Consider adding a brief comment above the example to explicitly point out the inclusion of the new optional remote source metadata ('name' and 'url') to draw attention to this feature.
Line range hint
367-421
: Comprehensive explanation of theload_model()
function!This section provides clear and accurate information about the
load_model()
function, including its purpose, implementation details, and support for optional keyword arguments. The guidance is valuable for users creating remote model sources.Consider adding a brief example of how optional keyword arguments might be used in a
load_model()
function to illustrate this advanced feature more concretely.
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
📒 Files selected for processing (1)
- docs/source/model_zoo/remote.rst (1 hunks)
🧰 Additional context used
🔇 Additional comments (2)
docs/source/model_zoo/remote.rst (2)
Line range hint
1-277
: Excellent documentation for remotely-sourced zoo models!This section provides comprehensive guidance on working with remotely-sourced zoo models. The documentation is well-structured, covering all major operations (registering, listing, downloading, applying, and deleting models) with both Python and CLI examples. This approach caters to different user preferences and skill levels.
Line range hint
334-365
: Clear explanation of thedownload_model()
function!This section provides accurate and helpful information about the
download_model()
function, including when it's needed and its purpose. The provided code snippet and explanation offer good guidance for implementing this function in a remote model source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM 👍
Summary by CodeRabbit