Fix image renaming collisions by appending UUID in mangle_image_name #8791
Conversation
WalkthroughThe changes in the pull request primarily involve modifications to the Changes
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
🧹 Outside diff range and nitpick comments (1)
cvat/apps/dataset_manager/bindings.py (1)
1901-1903: Simplify the UUID-based name generation logicThe implementation successfully uses UUIDs to prevent name collisions, but it can be simplified:
- The counter variable
iis incremented but not used in the UUID generation.- The loop with
sys.maxsizecheck is unnecessary since UUID collisions are extremely unlikely.Consider simplifying the code:
- i = 1 uid = uuid.uuid4().hex - while i < sys.maxsize: + new_image_name = f"{image_name}_{uid}" + if not names[(subset, new_image_name)]: + names[(subset, name)] += 1 + return osp.extsep.join([new_image_name, ext]) - new_image_name = f"{image_name}_{uid}" - if not names[(subset, new_image_name)]: - names[(subset, name)] += 1 - return osp.extsep.join([new_image_name, ext]) - i += 1If you want to keep collision detection, consider generating a new UUID instead of incrementing
i:while True: uid = uuid.uuid4().hex new_image_name = f"{image_name}_{uid}" if not names[(subset, new_image_name)]: names[(subset, name)] += 1 return osp.extsep.join([new_image_name, ext])
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
cvat/apps/dataset_manager/bindings.py(2 hunks)
🔇 Additional comments (1)
cvat/apps/dataset_manager/bindings.py (1)
11-11: LGTM!
The uuid import is correctly placed with other standard library imports and is necessary for generating unique identifiers.
|
|
The proposed changes aim to resolve image name conflicts by appending a UUID to the image name in the Updated Behavior By appending a UUID ( Updated Code Changes Explained
Testing the Fix
|
|
Hi, answered in #8076 (comment) |
Instead of appending an incremental integer to the end of files with colliding names we append a unique identifier, uuid.
Motivation and context
#8076
This change is required because currently, when you try to export a project that contains multiple tasks with the same image name, some can be overwritten on export.
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