fixing problems with skeleton tracks with missing points #8377
Conversation
|
Important Review skippedAuto 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 changes involve modifications to the annotation processing logic in the CVAT codebase. Key updates include adding filtering steps to prevent errors when sorting annotations, enhancing safety checks in functions handling shapes, and introducing new entries in JSON files to support skeleton tracks. Additionally, the test suite has been refactored to improve annotation creation logic and to add tests for new functionalities. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant AnnotationManager
participant AnnotationProcessor
User->>AnnotationManager: Create task
AnnotationManager->>AnnotationProcessor: Process annotations
AnnotationProcessor->>AnnotationProcessor: Filter out falsy values
AnnotationProcessor->>AnnotationProcessor: Sort valid annotations
AnnotationProcessor->>AnnotationManager: Return processed annotations
Assessment against linked issues
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
CodeRabbit Configuration File (
|
![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
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files selected for processing (5)
- cvat-core/src/annotations-objects.ts (1 hunks)
- cvat/apps/dataset_manager/annotation.py (2 hunks)
- cvat/apps/dataset_manager/tests/assets/annotations.json (1 hunks)
- cvat/apps/dataset_manager/tests/assets/tasks.json (1 hunks)
- cvat/apps/dataset_manager/tests/test_rest_api_formats.py (3 hunks)
Additional comments not posted (9)
cvat/apps/dataset_manager/tests/assets/tasks.json (1)
341-381: LGTM!The new entry "many jobs skeleton" is well-structured and consistent with the existing entries in the file.
cvat/apps/dataset_manager/tests/assets/annotations.json (1)
419-462: LGTM!The new entry "many jobs skeleton tracks with missing shapes" is well-structured and consistent with the existing entries in the file.
cvat/apps/dataset_manager/annotation.py (2)
94-94: LGTM!The change ensures that the function first verifies that
prev_shapeis notNonebefore attempting to access its properties, thereby preventing potential runtime errors whenprev_shapeis undefined.
555-556: LGTM!The addition of the guard clause enhances the robustness of the function by ensuring it handles cases where there are no shapes gracefully.
cvat/apps/dataset_manager/tests/test_rest_api_formats.py (4)
Line range hint
239-304: LGTM!The static method
_make_annotations_for_taskis correctly implemented and encapsulates the logic for creating annotations based on a task.
306-309: LGTM!The refactoring of the
_create_annotationsmethod to utilize the new static method_make_annotations_for_taskimproves code clarity and reduces redundancy.
312-314: LGTM!The refactoring of the
_create_annotations_in_jobmethod to call the new static method_make_annotations_for_taskimproves code clarity and reduces redundancy.
1266-1302: LGTM!The new test method
test_api_v2_check_skeleton_tracks_with_missing_shapesis comprehensive and covers various scenarios for validating the functionality of creating annotations in tasks with specific conditions.cvat-core/src/annotations-objects.ts (1)
2921-2921: LGTM!The change to filter out falsy values from the annotations array before sorting enhances the robustness of the sorting operation.
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## develop #8377 +/- ##
===========================================
+ Coverage 83.57% 83.59% +0.01%
===========================================
Files 396 396
Lines 41898 41900 +2
Branches 3881 3881
===========================================
+ Hits 35018 35028 +10
+ Misses 6880 6872 -8
|
|
Seems like SonarCloud Code Analysis is failing because of the addition of I suggest either refactoring this function further to meet the complexity criteria: Otherwise LGTM! |
Eldies
force-pushed
the
dl/skeleton_tracks_with_missing_shapes
branch
from
805e08b
to
8d7053a
Compare
refactored it |
|
<!-- Raise an issue to propose your change (https://github.com/cvat-ai/cvat/issues). It helps to avoid duplication of efforts from multiple independent contributors. Discuss your ideas with maintainers to be sure that changes will be approved and merged. Read the [Contribution guide](https://docs.cvat.ai/docs/contributing/). --> <!-- Provide a general summary of your changes in the Title above --> ### Motivation and context <!-- Why is this change required? What problem does it solve? If it fixes an open issue, please link to the issue here. Describe your changes in detail, add screenshots. --> fixing cvat-ai#7783 also fixing related problem - some jobs from the issue can not be opened in ui due to missing shapes ### How has this been tested? <!-- Please describe in detail how you tested your changes. Include details of your testing environment, and the tests you ran to see how your change affects other areas of the code, etc. --> ### Checklist <!-- Go over all the following points, and put an `x` in all the boxes that apply. If an item isn't applicable for some reason, then ~~explicitly strikethrough~~ the whole line. If you don't do that, GitHub will show incorrect progress for the pull request. If you're unsure about any of these, don't hesitate to ask. We're here to help! --> - [ ] I submit my changes into the `develop` branch - [ ] I have created a changelog fragment <!-- see top comment in CHANGELOG.md --> - [ ] I have updated the documentation accordingly - [ ] I have added tests to cover my changes - [ ] I have linked related issues (see [GitHub docs]( https://help.github.com/en/github/managing-your-work-on-github/linking-a-pull-request-to-an-issue#linking-a-pull-request-to-an-issue-using-a-keyword)) - [ ] I have increased versions of npm packages if it is necessary ([cvat-canvas](https://github.com/cvat-ai/cvat/tree/develop/cvat-canvas#versioning), [cvat-core](https://github.com/cvat-ai/cvat/tree/develop/cvat-core#versioning), [cvat-data](https://github.com/cvat-ai/cvat/tree/develop/cvat-data#versioning) and [cvat-ui](https://github.com/cvat-ai/cvat/tree/develop/cvat-ui#versioning)) ### License - [ ] I submit _my code changes_ under the same [MIT License]( https://github.com/cvat-ai/cvat/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. <!-- This is an auto-generated comment: release notes by coderabbit.ai --> ## Summary by CodeRabbit - **New Features** - Enhanced data integrity in annotation processing by adding filtering steps before sorting. - New entries for "many jobs skeleton tracks with missing shapes" and "many jobs skeleton" added to the dataset, improving job tracking capabilities and complex job definitions. - Introduced a new static method for annotation creation, improving code modularity and reusability. - **Bug Fixes** - Improved error handling in annotation functions to prevent runtime errors when accessing potentially undefined variables. - **Tests** - Expanded the testing framework with new tests for annotation creation scenarios, ensuring comprehensive coverage. <!-- end of auto-generated comment: release notes by coderabbit.ai -->
Motivation and context
fixing #7783
also fixing related problem - some jobs from the issue can not be opened in ui due to missing shapes
How has this been tested?
Checklist
developbranch(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Tests