Skip to content

Add KaggleCoco and Bbox capability in KaggleImageCsv and KaggleImageTxt#1273

Merged
wonjuleee merged 12 commits intoopen-edge-platform:developfrom
wonjuleee:add_kaggle_det
Feb 28, 2024
Merged

Add KaggleCoco and Bbox capability in KaggleImageCsv and KaggleImageTxt#1273
wonjuleee merged 12 commits intoopen-edge-platform:developfrom
wonjuleee:add_kaggle_det

Conversation

@wonjuleee
Copy link
Copy Markdown
Contributor

@wonjuleee wonjuleee commented Feb 26, 2024

Summary

How to 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) 2023 Intel Corporation
#
# SPDX-License-Identifier: MIT

@wonjuleee wonjuleee requested review from a team as code owners February 26, 2024 06:19
@wonjuleee wonjuleee requested review from sooahleex and removed request for a team February 26, 2024 06:19
@codecov
Copy link
Copy Markdown

codecov bot commented Feb 26, 2024

Codecov Report

Attention: Patch coverage is 80.35714% with 22 lines in your changes are missing coverage. Please review.

Project coverage is 80.77%. Comparing base (a00b0b8) to head (de8412a).

Files Patch % Lines
src/datumaro/plugins/data_formats/kaggle/base.py 80.35% 11 Missing and 11 partials ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##           develop    #1273      +/-   ##
===========================================
+ Coverage    80.76%   80.77%   +0.01%     
===========================================
  Files          270      271       +1     
  Lines        30599    30664      +65     
  Branches      6179     6190      +11     
===========================================
+ Hits         24713    24770      +57     
- Misses        4499     4503       +4     
- Partials      1387     1391       +4     
Flag Coverage Δ
ubuntu-20.04_Python-3.10 80.76% <80.35%> (?)
windows-2022_Python-3.10 80.75% <80.35%> (-0.01%) ⬇️

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.

Copy link
Copy Markdown
Contributor

@sooahleex sooahleex left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall structure looks good to me. I just left some minor comments.

return [float(coord.strip()) for coord in coords]

def _load_annotations(self, datas: list, indices: Dict[str, int], bbox_flag: bool):
if "label" in indices:
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if "label" in indices:
label_index = indices.get("label", None)

It seems better to get label index or use default value to avoid check this twice.

Comment on lines +128 to +129
if "label" in columns:
indices.update({"label": df_fields.index(columns["label"])})
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if "label" in columns:
indices.update({"label": df_fields.index(columns["label"])})
label_index = columns.get("label")
if label_index:
indices["label"] = df_fields.index(label_index)

@review-notebook-app
Copy link
Copy Markdown

Check out this pull request on  ReviewNB

See visual diffs & provide feedback on Jupyter Notebooks.


Powered by ReviewNB

@wonjuleee wonjuleee merged commit 8c7728e into open-edge-platform:develop Feb 28, 2024
@yunchu yunchu added this to the 2.0.0 milestone Mar 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants