Add train, val, test folder paths in data.yaml at save_data_yaml() - #1422
Add train, val, test folder paths in data.yaml at save_data_yaml()#1422xaristeidou wants to merge 8 commits into
train, val, test folder paths in data.yaml at save_data_yaml()#1422Conversation
|
Hi @xaristeidou 👋🏻! Thanks for staying active in the supervision repo! As for the PR you opened, the main problem I see is that YOLOv8 shares a single |
|
@SkalskiP Hello there 👋🏻! Yes indeed, if we want to create train, valid, test subset folders for the dataset training we have to run the process one time for each subset. Nevertheless, all 3 subset folder datasets contain the same I think there are 3 scenarios in that case:
|
|
@xaristeidou, One idea I had is that |
|
@SkalskiP Well not too complicated. We could modify
The function should modify the |
|
@xaristeidou, wanna try to implement the PoC of this solution? |
|
@SkalskiP Yes! I will be back when it is ready. |
|
@SkalskiP I have added the new changes. Here is a Colab notebook for easy testing. https://colab.research.google.com/drive/1BL7c2ycXkuCrEE5JOqh7u7Zf1kJzgb43?usp=sharing |
|
@SkalskiP Did you manage to take a look at the new committed changes and test with the notebook? |
|
@SkalskiP @onuralpszr After a long break away from the repository, I wanted to close some issues and PR's that are left open. As I see the feature we discussed hasn't been modified so I would like to ask if we are going to add that feature to include the path for each subset of a yolo dataset. I have merged all changes from https://colab.research.google.com/drive/1BL7c2ycXkuCrEE5JOqh7u7Zf1kJzgb43?usp=sharing Let me know if we will merge the feature in the repository otherwise we should close PR and issue. Thanks a lot! |
There was a problem hiding this comment.
Pull request overview
This PR aims to make YOLO dataset exports more “train-ready” by ensuring the generated data.yaml includes dataset subset paths (train, val, optionally test) required by Ultralytics YOLO training.
Changes:
- Adjust
save_data_yaml()to (attempt to) update an existingdata.yamlinstead of always overwriting from scratch. - Add a
subset_typeparameter toDetectionDataset.as_yolo()and plumb it through tosave_data_yaml(). - Minor docstring fixes in YAML/JSON file helpers.
Quality assessment (per repo guidelines):
- Code quality: 2/5
- Testing: 1/5
- Docs: 2/5
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
| File | Description |
|---|---|
supervision/utils/file.py |
Docstring tweaks for YAML/JSON helpers. |
supervision/dataset/formats/yolo.py |
Updates save_data_yaml() behavior (read/merge + subset path writing). |
supervision/dataset/core.py |
Extends as_yolo() API to pass subset_type into YAML generation. |
Comments suppressed due to low confidence (1)
supervision/dataset/core.py:517
- Adding
subset_typein the middle of theas_yolo()parameter list is a backward-incompatible API change for any callers using positional arguments (all later arguments shift by one). To avoid breaking existing code, append new optional parameters at the end of the signature.
def as_yolo(
self,
images_directory_path: Optional[str] = None,
annotations_directory_path: Optional[str] = None,
data_yaml_path: Optional[str] = None,
subset_type: Optional[str] = None,
min_image_area_percentage: float = 0.0,
max_image_area_percentage: float = 1.0,
approximation_percentage: float = 0.0,
) -> None:
| def save_json_file(data: dict, file_path: Union[str, Path], indent: int = 3) -> None: | ||
| """ | ||
| Write a dict to a json file. | ||
|
|
||
| Args: | ||
| indent: | ||
| data (dict): dict with unique keys and value as pair. | ||
| file_path (Union[str, Path]): The file path as a string or Path object. | ||
| """ |
| import os | ||
| from pathlib import Path | ||
| from typing import TYPE_CHECKING, Dict, List, Optional, Tuple | ||
| from typing import TYPE_CHECKING, Dict, List, Optional, Tuple, Union |
| def save_data_yaml( | ||
| data_yaml_path: str, classes: List[str], subset_type: Union[str, None] | ||
| ) -> None: | ||
| if Path(data_yaml_path).exists(): | ||
| data = read_yaml_file(data_yaml_path) | ||
| else: | ||
| data = {} | ||
| data["nc"] = len(classes) | ||
| data["names"] = classes | ||
| if subset_type is not None: | ||
| data[subset_type] = f"{subset_type}/images" | ||
| Path(data_yaml_path).parent.mkdir(parents=True, exist_ok=True) | ||
| save_yaml_file(data=data, file_path=data_yaml_path) |
| if data_yaml_path is not None: | ||
| save_data_yaml(data_yaml_path=data_yaml_path, classes=self.classes) | ||
| save_data_yaml( | ||
| data_yaml_path=data_yaml_path, | ||
| classes=self.classes, | ||
| subset_type=subset_type, | ||
| ) |
Description
In the process of developing a notebook as scheduled in #1388, I used the
sv.DetectionDataset().as_yolo()method which executes in the backed thesave_data_yaml()function to create thedata.yamlfile need for the dataset.As YOLO the model construction for training, and the documentation, the model needs prerequisite
train,valarguments in thedata.yamlfile, and thetestargument is not needed but could be passed also if a test dataset exists. When trying to runmodel.train()the following error raises:SyntaxError: /content/dataset/data.yaml 'train:' key missing ❌. 'train' and 'val' are required in all data YAMLs.Therefore in
save_data_yaml()except thenc,namesarguments we should export also thetrain,val,testpaths in order to be ready for executing themodel.train()process. I think we should export the default paths as follows:If someone has a different working directory than the root of the folder containing the data.yaml, should change these paths manually. At least it will be easier to debug and modify the path if needed than to add the arguments in the yaml file.
List any dependencies that are required for this change.
None
Please delete options that are not relevant.
How has this change been tested, please provide a testcase or example of how you tested the change?
Using
sv.DetectionDataset.as_yolo()exports the data.yaml file with prerequisitetrain, val, testpaths.Any specific deployment considerations
None
Docs