Fix slices handling in LazyList

[Bobronium]

Motivation and context

Following v2.16.2 release, we noticed an issue with annotations export with CVAT format and masks.

Upon investigation, we found that the problem stemmed from changes introduced in this PR: #8229.

The issue was caused by the improper handling of slices with negative positions in LazyList, particularly affecting the computation of resulting data. This behaviour was triggered by the following line:

cvat/cvat/apps/dataset_manager/formats/cvat.py

Line 797 in cfce7be

("rle", f"{list(int (v) for v in shape.points[:-4])}"[1:-1]),

How has this been tested?

Checklist

• I submit my changes into the develop branch
• I have created a changelog fragment
• I have updated the documentation accordingly
• I have added tests to cover my changes
• I have linked related issues (see GitHub docs)
• I have increased versions of npm packages if it is necessary
  (cvat-canvas,
  cvat-core,
  cvat-data and
  cvat-ui)

License

• I submit my code changes under the same MIT License that covers the project.
  Feel free to contact the maintainers if that's a concern.

Summary by CodeRabbit

• New Features

  • Enhanced handling of negative indices in list slicing, improving robustness and performance.
  • Added new test methods to validate slicing operations, ensuring correct behavior for various scenarios.

• Bug Fixes

  • Adjusted existing tests to account for empty values in the list, improving accuracy of expected outputs.

[coderabbitai]

Important

Review skipped

Auto incremental reviews are disabled on this repository.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Walkthrough

The changes enhance the functionality of the LazyList class by improving its handling of indexing and list parsing, particularly with negative indices. The tests for LazyList have been updated to reflect these enhancements, ensuring accurate handling of empty values and robust slicing behavior. Overall, these modifications contribute to a more reliable and maintainable implementation.

Changes

File	Change Summary
cvat/apps/engine/lazy_list.py	Enhanced __getitem__ method to handle negative indices better, ensuring full list parsing when necessary. Improved _iter_unparsed for clarity and performance.
cvat/apps/engine/tests/test_lazy_list.py	Updated test cases to accommodate new behaviors in LazyList, including handling of empty values, and added new tests for slicing operations.

Poem

🐇 In the garden of code, I hop with delight,
With slices and indices, everything feels right.
A list full of wonders, both long and quite small,
I’ll parse every value, and welcome them all!
So here’s to the changes, both clever and bright,
A joyful leap forward, in the soft moonlight! 🌙✨

---

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?

Share

• X
• Mastodon
• Reddit
• LinkedIn

Tips

Chat

There are 3 ways to chat with CodeRabbit:

• Review comments: Directly reply to a review comment made by CodeRabbit. Example:
  • I pushed a fix in commit <commit_id>.
  • Generate unit testing code for this file.
  • Open a follow-up GitHub issue for this discussion.
• Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
  • @coderabbitai generate unit testing code for this file.
  • @coderabbitai modularize this function.
• PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
  • @coderabbitai generate interesting stats about this repository and render them as a table.
  • @coderabbitai show all the console.log statements in this repository.
  • @coderabbitai read src/utils.ts and generate unit testing code.
  • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
  • @coderabbitai help me debug CodeRabbit configuration file.

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)

• @coderabbitai pause to pause the reviews on a PR.
• @coderabbitai resume to resume the paused reviews.
• @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
• @coderabbitai full review to do a full review from scratch and review all the files again.
• @coderabbitai summary to regenerate the summary of the PR.
• @coderabbitai resolve resolve all the CodeRabbit review comments.
• @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
• @coderabbitai help to get help.

Additionally, you can add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.

CodeRabbit Configuration File (.coderabbit.yaml)

• You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
• Please see the configuration documentation for more information.
• If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

• Visit our Documentation for detailed information on how to use CodeRabbit.
• Join our Discord Community to get help, request features, and share feedback.
• Follow us on X/Twitter for updates and announcements.

[coderabbitai]

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between 3b568b5 and dc2c822.

Files selected for processing (2)

• cvat/apps/engine/lazy_list.py (3 hunks)
• cvat/apps/engine/tests/test_lazy_list.py (2 hunks)

Additional comments not posted (7)

cvat/apps/engine/tests/test_lazy_list.py (5)

14-14: Initialization with empty values is appropriate.

The setUp method now initializes LazyList with a string containing empty values, which is suitable for testing edge cases.

---

22-24: Representation tests are accurate.

The test_repr method correctly verifies the representation of LazyList, accounting for both parsed and unparsed elements.

---

168-168: String conversion tests are accurate.

The test_str method correctly verifies the string conversion of LazyList, ensuring it handles both parsed and unparsed states.

---

176-185: Slicing tests are comprehensive.

The test_slice method effectively covers various slicing scenarios, including negative indices, ensuring correct behavior of LazyList.

---

187-191: Stride slicing tests are comprehensive.

The test_slice_with_stride method effectively covers slicing with various strides, ensuring correct behavior of LazyList.

cvat/apps/engine/lazy_list.py (2)

149-163: Enhanced handling of negative indices is robust.

The __getitem__ method now correctly handles negative indices by parsing the entire list when necessary, improving robustness.

---

Line range hint 211-221: Improved clarity and performance in _iter_unparsed.

The use of probable_length in _iter_unparsed reduces redundant calls and enhances readability and performance.

[zhiltsov-max]

Could you also update test assets or bracket handling in serialization into DB? I can see there are changes in array representations - the brackets are removed.

pytest --start-services --rebuild ./tests/python/
docker exec test_cvat_server_1 python manage.py dumpdata --indent 2 --natural-foreign --exclude=auth.permission --exclude=contenttypes --exclude=django_rq > tests/python/shared/assets/cvat_db/data.json

[zhiltsov-max]

• I suggest that the PR is renamed, as the problem is not directly related to the CVAT format
• Please update changelog

[codecov-commenter]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.35%. Comparing base (3b568b5) to head (47306f2).
Report is 94 commits behind head on develop.

Additional details and impacted files

@@             Coverage Diff             @@
##           develop    #8299      +/-   ##
===========================================
- Coverage    83.36%   83.35%   -0.02%     
===========================================
  Files          390      390              
  Lines        41498    41501       +3     
  Branches      3839     3839              
===========================================
- Hits         34596    34593       -3     
- Misses        6902     6908       +6     

Components	Coverage Δ
cvat-ui	79.61% <ø> (-0.04%)	⬇️
cvat-server	86.67% <100.00%> (+<0.01%)	⬆️

[sonarqubecloud]

Quality Gate passed

Issues
5 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud