[GSoC2024] Resolved issue #7523: Fixed error caused by filenames with same names #7602
Conversation
… same name but different extensions.
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## develop #7602 +/- ##
========================================
Coverage 83.47% 83.47%
========================================
Files 373 373
Lines 39739 39760 +21
Branches 3741 3741
========================================
+ Hits 33171 33191 +20
- Misses 6568 6569 +1
|
| dataset = Dataset.import_from(src_file.name, | ||
| 'coco_instances', env=dm_env) | ||
| #Reverse mapping names | ||
| for item in dataset: | ||
| item.id = reverse_no_ext_mapping[item.id] |
There was a problem hiding this comment.
Could you please try to implement the solution I described in #7523 (comment)? This code currently misses the most important part, that, in fact, should address the problem. Currently, as I see, the code doesn't change the situation, as it just modifies the input dataset and then changes everything back on the Datumaro side. As the problem is on the Datumaro side, it's not possible to solve it just by changing the Datumaro dataset object. Please also check the zip archive case as well, and then COCO Keypoints format in this file, it will have the same implementation for the fix.
There was a problem hiding this comment.
I've been experimenting with the implementation provided in #7523(comment), and I encountered a circular import error while attempting to load the rename_mapping variable into bindings.py.
ImportError: cannot import name 'GetCVATDataExtractor' from partially initialized module 'cvat.apps.dataset_manager.bindings' (most likely due to a circular import) (/home/adkbbx/projects/CVAT/cvat/cvat/apps/dataset_manager/bindings.py)
Which I thought would be the way of implementing this logic please do correct me if I was wrong with the implementation.
On furthur debugging I found that the
self._frame_mapping variable inside the bindings.py only contains the value {"image1": 1} may be since same keys were used since path "Image1" was same for both the images in the dataset and only the value of fram got update from 0->1 which I felt would be the cause of multiple annotations being placed on the same image on my current implementation.
To resolve this, I attempted to updating the _frame_mapping attribute with the instance_data and this seems to resolve the issue of multiple annotations being placed on the same image on my current implementation but while I was testing the export of datasets the annotations aren't getting updated into json file.
"annotations": []
So I think updating _frame_mapping might be the cause of this export error which I am unable to debug.
I would appreciate any hints, insights or suggestions for the implementation of this solution further. I'm sorry for the frequent contact and thank you for your time and patience! 🙇
There was a problem hiding this comment.
I've been experimenting with the implementation provided in #7523 (comment), and I encountered a circular import error while attempting to load the rename_mapping variable into bindings.py.
ImportError: cannot import name 'GetCVATDataExtractor' from partially initialized module 'cvat.apps.dataset_manager.bindings' (most likely due to a circular import) (/home/adkbbx/projects/CVAT/cvat/cvat/apps/dataset_manager/bindings.py)
Which I thought would be the way of implementing this logic please do correct me if I was wrong with the implementation.
The suggestion in the comment was to add a new function parameter to import_dm_annotations() in bindings.py. I believe, this doesn't require importing any modules. Then, you can supply the constructed mapping into this function, when it's called in coco.py. After this, in import_dm_annotations(), the match_dm_item() function gets called. It also needs a new parameter. match_dm_item() needs to be updated to use this new parameter to map the item name back into the original one, and find the matching frame id in the job or task.
There was a problem hiding this comment.
@zhiltsov-max
I've pushed a new commit attempting to address the issue and think I was able to incorporated the suggestions you provided earlier, including adding a new parameter to import_dm_annotations() in bindings.py and updating match_dm_item() accordingly. Despite these changes, It seems that the annotations are still being duplicated on some images even though the anotations are being accepted now. As of what I am able to understand from my debugging the way instance_dataset._frame_mapping updates when there are images with same name and different extensions might be the cause of this issue.
Any further guidance you can provide would be greatly appreciated. Thank you for your continued support and feedback.
There was a problem hiding this comment.
That's very nice to see advances in solving this problem.
As of what I am able to understand from my debugging the way
instance_dataset._frame_mappingupdates when there are images with same name and different extensions might be the cause of this issue.
Yeah, it seems to be the issue now. From what I've checked, we can try to change what's stored in these frame mappings, as they seem to only be used for frame matching. Probably, we can include file extensions in these mappings as well.
There was a problem hiding this comment.
I was able to get the inital tests setup and am able to run the tests sucessfully but using pytest ./tests/python and am getting theese results where some test cases are failing as well as status code 500 error which I think is an internal server error and am a little unsure about how to deal with this any pointers would be helpful.
Please try to check server logs with docker logs test_cvat_server_1, docker logs test_cvat_worker_import_1, docker logs test_cvat_worker_export_1. You can also attach a debugger, if it's needed.
please note that I came up with this refering to the current implementation of the test so do let me know if it is incorrect. Thank you for your support!
- Seems like it does quite a few unnecessary actions. Please consider minimizing the amount of actions done in the test setup.
- Importing the exported dataset makes no sense, as it doesn't reproduce the problem (since the export is broken too wrt. this issue).
- Why the test is checking that there are no annotations in the task after uploading?
There was a problem hiding this comment.
@zhiltsov-max I am sorry I think that was a mistake from me I will update the tests and check the logs as requested thank you!
There was a problem hiding this comment.
@zhiltsov-max I am sorry for the delayed response and must inform you that progress on this task has been slower than anticipated since I had been a little busy with the proposal submission for GSOC. I plan to complete working on this issue as soon as possible. I apologize for any inconvenience this may have caused, and I assure you that I am dedicated to completing this task promptly and effectively. Thank you for your understanding and continued support. 🙇
There was a problem hiding this comment.
Ok, do as you see fits best to you, it's your PR after all. Just keep in mind the program deadlines, if you want it to be a part of GSoC.
nmanovic
changed the title
nmanovic
changed the title
|
Hi, any updates on this PR? |
|
Closing as there's no activity for 1 month. Feel free to reopen, if you want to continue. |
Resolved issue #7523 : Fixed error caused by filenames with the same name but different extensions.
Motivation and context
This commit addresses a bug where uploading coco annotation files with the same image name but different extensions resulted in an error. The fix ensures that such files are processed correctly without causing errors in the system.
Before fix:
After fix:
How has this been tested?
image1.pngandimage1.jpg.Checklist
developbranchI have created a changelog fragmentI have updated the documentation accordinglyI have added tests to cover my changesI have increased versions of npm packages if it is necessary(cvat-canvas,
cvat-core,
cvat-data and
cvat-ui)
License
Feel free to contact the maintainers if that's a concern.