-
Notifications
You must be signed in to change notification settings - Fork 2.9k
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
Fix automatic tag
annotation support
#7839
Fix automatic tag
annotation support
#7839
Conversation
Important Review skippedAutomatic incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe recent changes introduce the Changes
Sequence Diagram(s) (Beta)sequenceDiagram
participant User
participant ToolsControlComponent
participant DetectedShape
User ->> ToolsControlComponent: Selects a tool
ToolsControlComponent ->> DetectedShape: Creates/Updates shape with objectType
DetectedShape ->> ToolsControlComponent: Shape created/updated with objectType
ToolsControlComponent ->> User: Displays updated shape
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 as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
tag
annotation supporttag
annotation support
May you please add changelog entry? Fixed
Example files may be found here: https://github.com/cvat-ai/cvat/tree/develop/changelog.d |
@bsekachev changelog entry added |
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: 2
Outside diff range and nitpick comments (2)
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx (2)
Line range hint
75-75
: Specify explicit types instead ofany
.Consider replacing
any
types with more specific types to improve type safety and code maintainability. TypeScript'sany
type defeats the purpose of using TypeScript in the first place, as it does not enforce any checking. Here are some lines whereany
is used:
- Line 75
- Line 76
- Line 138
- Line 157
- Line 189
- Line 441
- Line 497
- Line 534
- Line 596
- Line 678
- Line 787
- Line 830
- Line 841
- Line 900
- Line 976
- Line 1273
- Line 1328
- Line 1333
Also applies to: 76-76, 138-138, 157-157, 189-189, 441-441, 497-497, 534-534, 596-596, 678-678, 787-787, 830-830, 841-841, 900-900, 976-976, 1273-1273, 1328-1328, 1333-1333
Line range hint
5-6
: Remove unused imports.- import React, { ReactPortal } from 'react'; + import React from 'react';The
ReactPortal
import is only used as a type and should be imported in a type-specific manner to avoid confusion and potential side effects in the bundle size.
Review Details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (2)
- changelog.d/20240528_180958_omer.sarioglu_semi_auto_tag_annotation.md (1 hunks)
- cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx (1 hunks)
Additional Context Used
Markdownlint (2)
changelog.d/20240528_180958_omer.sarioglu_semi_auto_tag_annotation.md (2)
1: Expected: 0 or 2; Actual: 1
Trailing spaces
4: null
Bare URL used
Biome (20)
cvat-ui/src/components/annotation-page/standard-workspace/controls-side-bar/tools-control.tsx (20)
75-75: Unexpected any. Specify a different type.
76-76: Unexpected any. Specify a different type.
138-138: Unexpected any. Specify a different type.
157-157: Unexpected any. Specify a different type.
189-189: Unexpected any. Specify a different type.
189-189: Unexpected any. Specify a different type.
441-441: Unexpected any. Specify a different type.
497-497: Unexpected any. Specify a different type.
534-534: Unexpected any. Specify a different type.
596-596: Unexpected any. Specify a different type.
678-678: Unexpected any. Specify a different type.
787-787: Unexpected any. Specify a different type.
830-830: Unexpected any. Specify a different type.
841-841: Unexpected any. Specify a different type.
900-900: Unexpected any. Specify a different type.
976-976: Unexpected any. Specify a different type.
1273-1273: Unexpected any. Specify a different type.
1328-1328: Unexpected any. Specify a different type.
1333-1333: Unexpected any. Specify a different type.
5-6: Some named imports are only used as types.
changelog.d/20240528_180958_omer.sarioglu_semi_auto_tag_annotation.md
Outdated
Show resolved
Hide resolved
changelog.d/20240528_180958_omer.sarioglu_semi_auto_tag_annotation.md
Outdated
Show resolved
Hide resolved
…tion.md Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Hello, To be consistent with our enterprise code, I modified the approach a little bit. Just need to properly handle |
Anyway, thanks for the contribution! |
Motivation and context
Since the lambda function's return
objectType
is always assumed to beSHAPE
, thetag
object cannot be passed, and it throws an error like in #6220 (comment).Adding
"objectType": "tag"
next to"type": "tag"
key-value pair to the lambda function fixed this problem.How has this been tested?
Classifier serverless function PR will be sent too.
Checklist
develop
branch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit