Apply clean transform of updated annotations only for tabular annotation type #1533

sooahleex · 2024-06-17T04:54:27Z

Summary

Fix to update annotation for Tabular and Caption annotation type during Clean transform
Add comment for Clean
Fix typo of function name

How to test

Update unit test

Checklist

I have added unit tests to cover my changes.
I have added integration tests to cover my changes.
I have added the description of my changes into CHANGELOG.
I have updated the documentation accordingly

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.
I have updated the license header for each file (see an example below).

# Copyright (C) 2024 Intel Corporation
#
# SPDX-License-Identifier: MIT

codecov · 2024-06-17T07:54:50Z

Codecov Report

Attention: Patch coverage is 73.68421% with 5 lines in your changes missing coverage. Please review.

Please upload report for BASE (releases/1.8.0@35e87c2). Learn more about missing BASE report.

Files	Patch %	Lines
src/datumaro/components/dataset.py	25.00%	2 Missing and 1 partial ⚠️
src/datumaro/plugins/transforms.py	86.66%	1 Missing and 1 partial ⚠️

Additional details and impacted files

@@                Coverage Diff                @@
##             releases/1.8.0    #1533   +/-   ##
=================================================
  Coverage                  ?   81.05%           
=================================================
  Files                     ?      276           
  Lines                     ?    32373           
  Branches                  ?     6580           
=================================================
  Hits                      ?    26239           
  Misses                    ?     4687           
  Partials                  ?     1447

Flag	Coverage Δ
ubuntu-20.04_Python-3.10	`81.04% <73.68%> (?)`
windows-2022_Python-3.10	`81.02% <73.68%> (?)`

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

### Summary CVS-143460  ### How to test  ### Checklist  - [x] I have added unit tests to cover my changes. - [ ] I have added integration tests to cover my changes. - [ ] I have added the description of my changes into [CHANGELOG](https://github.com/openvinotoolkit/datumaro/blob/develop/CHANGELOG.md). - [ ] I have updated the [documentation](https://github.com/openvinotoolkit/datumaro/tree/develop/docs) accordingly ### License - [x] I submit _my code changes_ under the same [MIT License](https://github.com/openvinotoolkit/datumaro/blob/develop/LICENSE) that covers the project. Feel free to contact the maintainers if that's a concern. - [ ] I have updated the license header for each file (see an example below). ```python # Copyright (C) 2024 Intel Corporation # # SPDX-License-Identifier: MIT ``` --------- Signed-off-by: Ilya Trushkin <ilya.trushkin@intel.com>

…t#1528)" This reverts commit c296000.

vinnamkim · 2024-06-25T01:19:18Z

src/datumaro/plugins/transforms.py

+                    for key in refined_media.data.keys()
+                    if ann.caption.startswith(key)
+                ]
+                ann = ann.wrap(caption=value[0])


Is it guaranteed that value has always one element?

As we get value through the above lines, caption would have only one value.

value comes from this for-loop for key in refined_media.data.keys(). How did you guarantee it is singular?

So far, the expectation is to do AstypeAnnotations on a tabular dataset before the Clean transformation. So if the annotation type is Caption, then it should have only one of them. We've refined the tabular annotation, so if "Title" is "It's really good!", we've transformed it to "Title:It's really good! We have not yet covered other normal image datasets that are not refined tabular datasets. So if the annotation type is caption, it should be singular. To cover other normal image datasets, I will come back to this.

That seems an implicit condition in Datumaro. What I want is to make everything explicit if possible. Relying on the implicit condition can lead to a bug. If someone develops something that can changes the implicit condition, he must never be noticed what he is doing can affect to the behavior of this code. It leads to a bad result in software maintenance. Add something like #1533 (comment)

That's why I added https://github.com/sooahleex/datumaro/blob/f6d5c3902d59e7f2d84ad8ef74edf3dfad2c9da6/src/datumaro/plugins/transforms.py#L1979-L1982. If this is not enough to show the implicit condition, please let me know. I will consider it.

vinnamkim · 2024-06-25T01:20:29Z

src/datumaro/plugins/transforms.py

+        for ann in item.annotations:
+            if isinstance(ann, Tabular):
+                annotation_values = {
+                    key: refined_media.data[key] for key in item.annotations[0].values.keys()


What's the reason that we do not care item.annotations[i].values for i=1,... not just i=0?

If a dataset item contains a tabular annotation type, it will contain only one annotation of tabular for each item. This item will not contain any other annotations. I think this part is a bit hard-coded, so if there is another way to check the annotation type, please let me know.

vinnamkim · 2024-06-25T02:08:51Z

src/datumaro/plugins/transforms.py

+            if isinstance(ann, Tabular):
+                annotation_values = {


You might be able to put in a kind of checker here such as

Suggested change

if isinstance(ann, Tabular):

annotation_values = {

if isinstance(ann, Tabular):

if len(item.annotations) != 1:

msg = ...

raise ValueError(msg)

annotation_values = {

I added this through afd56c5. Thank you for your suggestion.

sooahleex marked this pull request as ready for review June 17, 2024 07:46

sooahleex requested review from a team as code owners June 17, 2024 07:46

sooahleex requested review from vinnamkim and removed request for a team June 17, 2024 07:46

sooahleex mentioned this pull request Jun 17, 2024

Add notebook for data handling of kaggle dataset #1534

Merged

6 tasks

sooahleex added this to the 1.8.0 milestone Jun 21, 2024

sooahleex changed the base branch from develop to releases/1.8.0 June 21, 2024 09:07

itrushkin and others added 6 commits June 22, 2024 06:50

Apply clean transform of updated annotations only for tabular ann

b0bc72c

Cover caption

8d89e59

Revert "Add hierarchical ImageNet-like dataset format (openvinotoolki…

b7b7ffb

…t#1528)" This reverts commit c296000.

Check annotation length for tabular annotation type

afd56c5

Fix typo

f6d5c39

sooahleex force-pushed the bugfix/clean_transform_for_tabular_ann branch from 9ccf4a2 to 8d89e59 Compare June 24, 2024 07:59

vinnamkim reviewed Jun 25, 2024

View reviewed changes

vinnamkim approved these changes Jun 25, 2024

View reviewed changes

sooahleex merged commit 6a50cdb into openvinotoolkit:releases/1.8.0 Jun 25, 2024
8 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Apply clean transform of updated annotations only for tabular annotation type #1533

Apply clean transform of updated annotations only for tabular annotation type #1533

sooahleex commented Jun 17, 2024 •

edited

Loading

codecov bot commented Jun 17, 2024 •

edited

Loading

vinnamkim Jun 25, 2024

sooahleex Jun 25, 2024

vinnamkim Jun 25, 2024

sooahleex Jun 25, 2024 •

edited

Loading

vinnamkim Jun 25, 2024 •

edited

Loading

sooahleex Jun 25, 2024

vinnamkim Jun 25, 2024

sooahleex Jun 25, 2024

vinnamkim Jun 25, 2024

sooahleex Jun 25, 2024

Apply clean transform of updated annotations only for tabular annotation type #1533

Apply clean transform of updated annotations only for tabular annotation type #1533

Conversation

sooahleex commented Jun 17, 2024 • edited Loading

Summary

How to test

Checklist

License

codecov bot commented Jun 17, 2024 • edited Loading

Codecov Report

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sooahleex Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

vinnamkim Jun 25, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

sooahleex commented Jun 17, 2024 •

edited

Loading

codecov bot commented Jun 17, 2024 •

edited

Loading

sooahleex Jun 25, 2024 •

edited

Loading

vinnamkim Jun 25, 2024 •

edited

Loading