-
Notifications
You must be signed in to change notification settings - Fork 12
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
YOLOv8 classification #59
Conversation
Important Review skippedAuto incremental reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThe pull request introduces support for the YOLOv8 Classification format across multiple components of the Datumaro framework. Key changes include the addition of new classes for importing, exporting, and extracting datasets in the YOLOv8 Classification format. The changelog has been updated to reflect these enhancements, and comprehensive documentation has been created to guide users in utilizing the new format. Additionally, unit tests have been implemented to ensure functionality and robustness of the new features. 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 (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
Outside diff range and nitpick comments (8)
site/content/en/docs/formats/yolo_v8_classification.md (5)
7-14
: Clearly specifies the supported annotation type and attributes.The section provides a helpful link to the format specification and clearly states that the format supports
Label
annotations without any attributes.Please fix the grammatical number in this sentence:
-Format doesn't support any attributes for annotations objects. +Format doesn't support any attributes for annotation objects.Tools
LanguageTool
[uncategorized] ~14-~14: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...rmat doesn't support any attributes for annotations objects. ## Import YOLOv8 classificat...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
17-52
: Provides clear instructions for importing datasets.The section offers helpful guidance on importing YOLOv8 Classification datasets using both the command line and Python API, along with the expected directory structure.
Please make the following changes:
-A Datumaro project with a ImageNet dataset can be created +A Datumaro project with an ImageNet dataset can be created-For successful importing of YOLOv8 Classification dataset the input directory with dataset +For successful importing of the YOLOv8 Classification dataset, the input directory with the dataset -should has the following structure: +should have the following structure:Tools
LanguageTool
[uncategorized] ~19-~19: Use the indefinite article “an” before nouns that start with a vowel sound.
Context: ...cation dataset A Datumaro project with a ImageNet dataset can be created in the ...(AI_EN_LECTOR_REPLACEMENT_DETERMINER_A_AN)
[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...tion') ``` For successful importing of YOLOv8 Classification dataset the input direct...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...sful importing of YOLOv8 Classification dataset the input directory with dataset should...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~36-~36: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...the input directory with dataset should has the following structure: ```bash datas...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
54-83
: Provides clear instructions for exporting datasets.The section offers helpful guidance on exporting YOLOv8 Classification datasets to other formats supported by Datumaro, using both the command line and Python API. The note about extra export options for some formats, along with the link to format-specific documentation, is useful.
Please add a comma in this sentence:
-For particular format see the +For a particular format, see theTools
LanguageTool
[uncategorized] ~72-~72: You might be missing the article “the” here.
Context: ...our YOLOv8 Classification dataset using Python API ```python import datumaro as dm d...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~82-~82: Possible missing comma found.
Context: ...ve extra export options. For particular format see the > docs to get...(AI_HYDRA_LEO_MISSING_COMMA)
85-99
: Provides clear instructions for converting datasets to YOLOv8 Classification format.The section offers helpful guidance on converting datasets containing
Label
annotations to the YOLOv8 Classification format using Datumaro, with examples for both the command line and a Datumaro project.Please add a comma before "and" in this sentence:
-If your dataset contains `Label` for images and you want to convert this +If your dataset contains `Label` for images, and you want to convert thisTools
LanguageTool
[uncategorized] ~87-~87: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...your dataset containsLabel
for images and you want to convert this dataset into t...(COMMA_COMPOUND_SENTENCE)
101-105
: Clearly lists and explains extra export options.The section provides a clear list of extra options for exporting to YOLOv8 Classification formats, along with brief explanations of each option.
Please rephrase these sentences to fix the grammatical issues:
-- `--save-media` allow to export dataset with saving media files +- `--save-media` allows exporting the dataset with media files (by default `False`) -- `--save-dataset-meta` - allow to export dataset with saving dataset meta +- `--save-dataset-meta` allows exporting the dataset with dataset metadata file (by default `False`)Tools
LanguageTool
[grammar] ~102-~102: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...ication formats: ---save-media
allow to export dataset with saving media files (by d...(ALLOW_TO)
[grammar] ~104-~104: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...False) -
--save-dataset-meta` - allow to export dataset with saving dataset meta file...(ALLOW_TO)
datumaro/plugins/yolo_format/converter.py (1)
409-443
: Theapply
method looks good overall!The method follows a clear structure:
- Validates the media type.
- Creates necessary directories.
- Saves dataset metadata if required.
- Iterates through subsets and items.
- Calls
_export_media_for_label
based on item annotations.A few minor suggestions:
- Consider adding a comment to explain the purpose of the
DEFAULT_SUBSET_NAME
check at line 425.- The
assert
statement at line 429 could be replaced with a more informative error message if the condition is not met.tests/unit/data_formats/test_yolo_format.py (2)
1001-1015
: Add docstrings to empty overridden test methodsThe methods
test_export_rotated_bbox
,test_cant_save_with_reserved_subset_name
,test_inplace_save_writes_only_updated_data
,test_can_load_dataset_with_exact_image_info
, andtest_can_save_and_load_without_path_prefix
are overridden with empty bodies. Providing docstrings explaining why these methods are intentionally left empty will improve code readability and help other developers understand their purpose.
978-1000
: Consistent handling of overridden methods with different argumentsThe method
test_can_save_and_load_image_with_arbitrary_extension
overrides a base class method with a different signature but does not disable thepylint
warning forarguments-differ
. For consistency and to avoid linting issues, consider adding# pylint: disable=arguments-differ
or aligning the method signature with the base class.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
tests/assets/yolo_dataset/yolov8_classification/train/label_0/2.jpg
is excluded by!**/*.jpg
tests/assets/yolo_dataset/yolov8_classification/train/label_0/subfolder/1.jpg
is excluded by!**/*.jpg
tests/assets/yolo_dataset/yolov8_classification/train/label_1/3.jpg
is excluded by!**/*.jpg
Files selected for processing (8)
- CHANGELOG.md (1 hunks)
- datumaro/plugins/yolo_format/converter.py (2 hunks)
- datumaro/plugins/yolo_format/extractor.py (9 hunks)
- datumaro/plugins/yolo_format/format.py (1 hunks)
- datumaro/plugins/yolo_format/importer.py (3 hunks)
- site/content/en/docs/formats/yolo_v8_classification.md (1 hunks)
- site/content/en/docs/user-manual/supported_formats.md (1 hunks)
- tests/unit/data_formats/test_yolo_format.py (7 hunks)
Additional context used
LanguageTool
site/content/en/docs/formats/yolo_v8_classification.md
[uncategorized] ~14-~14: The grammatical number of this noun doesn’t look right. Consider replacing it.
Context: ...rmat doesn't support any attributes for annotations objects. ## Import YOLOv8 classificat...(AI_EN_LECTOR_REPLACEMENT_NOUN_NUMBER)
[uncategorized] ~19-~19: Use the indefinite article “an” before nouns that start with a vowel sound.
Context: ...cation dataset A Datumaro project with a ImageNet dataset can be created in the ...(AI_EN_LECTOR_REPLACEMENT_DETERMINER_A_AN)
[uncategorized] ~35-~35: You might be missing the article “the” here.
Context: ...tion') ``` For successful importing of YOLOv8 Classification dataset the input direct...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~35-~35: A comma might be missing here.
Context: ...sful importing of YOLOv8 Classification dataset the input directory with dataset should...(AI_EN_LECTOR_MISSING_PUNCTUATION_COMMA)
[uncategorized] ~36-~36: This verb does not appear to agree with the subject. Consider using a different form.
Context: ...the input directory with dataset should has the following structure: ```bash datas...(AI_EN_LECTOR_REPLACEMENT_VERB_AGREEMENT)
[uncategorized] ~72-~72: You might be missing the article “the” here.
Context: ...our YOLOv8 Classification dataset using Python API ```python import datumaro as dm d...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[uncategorized] ~82-~82: Possible missing comma found.
Context: ...ve extra export options. For particular format see the > docs to get...(AI_HYDRA_LEO_MISSING_COMMA)
[uncategorized] ~87-~87: Use a comma before ‘and’ if it connects two independent clauses (unless they are closely connected and short).
Context: ...your dataset containsLabel
for images and you want to convert this dataset into t...(COMMA_COMPOUND_SENTENCE)
[grammar] ~102-~102: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...ication formats: ---save-media
allow to export dataset with saving media files (by d...(ALLOW_TO)
[grammar] ~104-~104: Did you mean “exporting”? Or maybe you should add a pronoun? In active voice, ‘allow’ + ‘to’ takes an object, usually a pronoun.
Context: ...False) -
--save-dataset-meta` - allow to export dataset with saving dataset meta file...(ALLOW_TO)
Additional comments not posted (6)
datumaro/plugins/yolo_format/format.py (1)
31-32
: LGTM!The addition of the
YOLOv8ClassificationFormat
class and theIMAGE_DIR_NO_LABEL
constant enhances the structure of the code by providing support for the YOLOv8 classification format and a mechanism to handle images without labels.datumaro/plugins/yolo_format/importer.py (1)
114-123
: LGTM!The new
YOLOv8ClassificationImporter
class looks good:
- It correctly extends the
Importer
class.- The
find_sources
method implementation is logically sound and handles the cases appropriately.- The changes enhance the functionality of the YOLO format importer to support classification data.
Great job!
site/content/en/docs/user-manual/supported_formats.md (1)
181-184
: LGTM!The new section for the YOLOv8 classification format is well-structured and consistent with the rest of the document. The provided links will be helpful for users to understand and utilize this format effectively.
CHANGELOG.md (1)
50-51
: LGTM!The changelog entry is well-formatted, descriptive, and consistent with the feature addition mentioned in the summary. Great job!
datumaro/plugins/yolo_format/extractor.py (1)
691-701
: Verify Python version compatibility due to use of assignment expressionsThe code uses assignment expressions (the walrus operator
:=
) in the comprehension at line 699, which requires Python 3.8 or newer. If the project needs to support earlier Python versions, this will cause syntax errors. Please verify the minimum required Python version for the project and adjust accordingly.To confirm the project's Python version compatibility, run the following script:
Verification successful
Python version compatibility verified and consistent with codebase
The project's setup.py file specifies a minimum Python version of 3.8, which is appropriate for the use of assignment expressions (walrus operator). This requirement is consistent with the code in question and the usage of assignment expressions throughout the codebase, particularly in the yolo_format plugin.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for the minimum required Python version in project files # Expected result: Python version should be >= 3.8 due to usage of assignment expressions # Searching for 'python_requires' in 'setup.py' or 'pyproject.toml' grep -r "python_requires" setup.py pyproject.toml || echo "python_requires not specified." # If 'python_requires' is not specified, check for assignment expressions usage grep -r ".*:=.*" datumaro/ || echo "No assignment expressions found."Length of output: 973
tests/unit/data_formats/test_yolo_format.py (1)
1804-1820
: Verify the use of extractor name in dataset exportIn the
YOLOv8ClassificationExtractorTest.test_can_parse
method, the dataset is exported usingYOLOv8ClassificationExtractor.NAME
. Since this is an extractor test, ensure that using the extractor's name for export is intended and functions correctly. Typically, a converter's name is used when exporting datasets.
Could you resolve coderabbitai comments by ether applying suggestions or writing why they are not applicable? |
7e52b98
to
07734fe
Compare
07734fe
to
5495d24
Compare
Black is failing, but otherwise LGTM! |
|
||
|
||
class YOLOv8ClassificationConverter(Converter): | ||
# https://github.com/AlexeyAB/darknet#how-to-train-to-detect-your-custom-objects |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The url seems to be unrelated to YOLOv8. I think it would make sense to add a similar one for v8.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the url.
There is already a link to official docs for yolov8 in datumaro docs, so i dont see a point of adding a link to some unofficial guide here (also I did not manage to find something like this)
Quality Gate passedIssues Measures |
Summary
Adds support for yolo v8 classification format
How to test
Checklist
develop
branchLicense
Feel free to contact the maintainers if that's a concern.
Summary by CodeRabbit
New Features
Bug Fixes
Documentation