Fallback to mask targets when annotating existing segmentation fields #5318
Conversation
Segmentation annotations default flow modifications
WalkthroughThis pull request focuses on enhancing the documentation for annotation integrations across multiple platforms including CVAT, Labelbox, and Label Studio. The primary changes involve clarifying the behavior of Changes
Possibly related PRs
Suggested labels
Poem
Tip CodeRabbit's docstrings feature is now available as part of our Early Access Program! Simply use the command 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 (1)
docs/source/integrations/cvat.rst (1)
460-464: Documentation update looks good with a minor formatting suggestion.The clarification of
mask_targetsparameter behavior is accurate and helpful. Consider adding a line break before "For existing label fields" to improve readability.strings. Only applicable when annotating semantic segmentations. All new label fields must have mask targets provided via one of the supported methods. + For existing label fields, if mask targets are not provided by this argument nor label_schema, any applicable mask targets stored on your dataset will be used, if available
📜 Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
docs/source/integrations/cvat.rst(2 hunks)docs/source/integrations/labelbox.rst(2 hunks)docs/source/integrations/labelstudio.rst(2 hunks)docs/source/user_guide/annotation.rst(2 hunks)fiftyone/utils/annotations.py(3 hunks)
🔇 Additional comments (9)
fiftyone/utils/annotations.py (4)
165-169: Documentation clarity is excellent here
This expanded explanation of how mask_targets behaves for new vs. existing label fields is well-structured and accurate.
475-479: Ensure consistent usage of _existing_field in _get_mask_targets()
Passing existing_field to _get_mask_targets() helps differentiate the new vs. existing label field logic. No issues found, but double-check that flow elsewhere in the file references _existing_field properly.
852-858: Signature update looks correct
The addition of the existing_field parameter is consistent with its usage in _build_label_schema. This improves clarity for new label fields vs. existing ones.
863-875: Helpful fallback logic for mask targets
Making new label fields raise an error when no mask targets are provided is a good safeguard. The fallback to either dataset or default mask targets for existing fields is clearly implemented and aligns with the doc changes.
docs/source/integrations/labelstudio.rst (1)
447-451: Documentation update confirms mask target requirements
Stating that new label fields must have mask targets is consistent with other updates across the codebase. This clarifies usage for new vs. existing fields.
docs/source/user_guide/annotation.rst (2)
513-517: Clearer mask target requirement
The mention that new semantic segmentation fields must provide mask targets, and existing fields may use stored or default mask targets, is consistent with the code changes. Nicely done.
676-682: Improved documentation for omitted parameters
The updated explanation for the classes and mask_targets parameters is succinct and ensures users know that existing fields can infer these from dataset properties.
docs/source/integrations/labelbox.rst (2)
459-463: Aligns with newly introduced mask target behavior
These lines clarify that new fields require explicit mask targets, while existing fields can leverage dataset-level settings. This is consistent with other doc changes.
632-638: Refined documentation for omitted classes/mask targets
Communicating that classes and mask targets can be inferred for existing fields reduces user confusion and matches recently introduced fallback logic.
Merging #4990 plus some documentation updates.
Summary by CodeRabbit
mask_targetsandclassesparameters in the annotation process across CVAT, Labelbox, and Label Studio integrations.annotate()method, improving clarity on parameter usage.