diff --git a/roboflow/core/version.py b/roboflow/core/version.py index 4d8a9af0..0ac2294e 100644 --- a/roboflow/core/version.py +++ b/roboflow/core/version.py @@ -35,7 +35,7 @@ from roboflow.util.annotations import amend_data_yaml from roboflow.util.general import extract_zip, write_line from roboflow.util.model_processor import package_custom_weights_interactive, validate_model_type_for_project -from roboflow.util.versions import get_model_format, get_wrong_dependencies_versions +from roboflow.util.versions import get_model_format if TYPE_CHECKING: import numpy as np @@ -644,33 +644,62 @@ def __get_format_identifier(self, format): return friendly_formats.get(format, format) def __reformat_yaml(self, location: str, format: str): - """ - Certain formats seem to require reformatting the downloaded YAML. + """Reformat the downloaded ``data.yaml`` for specific YOLO formats. + + The ZIP downloaded from Roboflow contains a ``data.yaml`` whose + ``train`` / ``val`` / ``test`` paths are prefixed with ``../`` which + is incorrect — the split directories are siblings of ``data.yaml``, + not in the parent directory. This method rewrites those paths so + that training works out-of-the-box. + + * Modern formats (``yolov8``, ``yolov9``) use relative paths + (``train/images``, ``valid/images``, ``test/images``) as expected + by current ultralytics releases. + * Legacy formats (``yolov5pytorch``, ``yolov7pytorch``, + ``mt-yolov6``) use absolute paths constructed from *location*. + + The ``test`` key is only written when the version actually has a + test split (``self.splits["test"] > 0``). Args: - location (str): filepath of the data directory that contains the yaml file - format (str): the format identifier string - """ # noqa: E501 // docs + location: Filepath of the data directory that contains the yaml file. + format: The format identifier string. + """ data_path = os.path.join(location, "data.yaml") + def _strip_rel_prefix(path: str) -> str: + """Remove all leading ``../`` and ``./`` prefixes from *path*.""" + while True: + if path.startswith("../"): + path = path[3:] + elif path.startswith("./"): + path = path[2:] + else: + return path + def data_yaml_callback(content: dict) -> dict: - if format == "mt-yolov6": - content["train"] = location + content["train"].lstrip(".") - content["val"] = location + content["val"].lstrip(".") - content["test"] = location + content["test"].lstrip(".") - if format in ["yolov5pytorch", "yolov7pytorch"]: - content["train"] = location + content["train"].lstrip("..") - content["val"] = location + content["val"].lstrip("..") - try: - # get_wrong_dependencies_versions raises exception if ultralytics is not installed at all # noqa: E501 // docs - if format == "yolov8" and not get_wrong_dependencies_versions( - dependencies_versions=[("ultralytics", "==", "8.0.196")] - ): - content["train"] = "train/images" - content["val"] = "valid/images" + has_test_split = self.splits.get("test", 0) > 0 + + # Modern ultralytics formats — relative paths + if format in ("yolov8", "yolov9"): + content["train"] = "train/images" + content["val"] = "valid/images" + if has_test_split: content["test"] = "test/images" - except ModuleNotFoundError: - pass + elif "test" in content: + del content["test"] + return content + + # Legacy formats — absolute paths + if format in ("yolov5pytorch", "yolov7pytorch", "mt-yolov6"): + for key in ("train", "val", "test"): + if key in content: + if key == "test" and not has_test_split: + del content["test"] + else: + content[key] = os.path.join(location, _strip_rel_prefix(content[key])) + return content + return content if format in ["yolov5pytorch", "mt-yolov6", "yolov7pytorch", "yolov8", "yolov9"]: diff --git a/tests/test_version.py b/tests/test_version.py index 64b8874e..4f23dbc9 100644 --- a/tests/test_version.py +++ b/tests/test_version.py @@ -1,8 +1,10 @@ import os +import tempfile import unittest from unittest.mock import patch import responses +import yaml from roboflow.adapters import rfapi from roboflow.config import ( @@ -266,3 +268,199 @@ def test_detection_project_rejects_sem_model(self): def test_classification_project_rejects_detection(self): with self.assertRaises(ValueError): self._version(TYPE_CLASSICATION)._validate_against_project_type("yolov11") + + +# --------------------------------------------------------------------------- +# __reformat_yaml — fixing issue #240 (Incorrect Data Path in YOLOv8 Dataset) +# --------------------------------------------------------------------------- + +_INITIAL_YAML = { + "train": "../train/images", + "val": "../valid/images", + "test": "../test/images", + "nc": 3, + "names": ["cat", "dog", "bird"], + "roboflow": {"license": "MIT", "project": "test-project", "version": 1}, +} + + +def _write_data_yaml(directory: str, content: dict) -> str: + """Write a ``data.yaml`` file in *directory* and return its path.""" + path = os.path.join(directory, "data.yaml") + with open(path, "w") as fh: + yaml.dump(content, fh) + return path + + +def _read_data_yaml(directory: str) -> dict: + """Read the ``data.yaml`` file from *directory*.""" + with open(os.path.join(directory, "data.yaml")) as fh: + return yaml.safe_load(fh) + + +class TestReformatYaml(unittest.TestCase): + """Tests for ``Version.__reformat_yaml`` (issue #240).""" + + # ------------------------------------------------------------------ + # helpers + # ------------------------------------------------------------------ + + def _version_with_test(self): + return get_version(splits={"train": 210, "valid": 20, "test": 10}) + + def _version_without_test(self): + return get_version(splits={"train": 210, "valid": 20, "test": 0}) + + def _reformat(self, version, location, fmt): + """Invoke the name-mangled private method.""" + version._Version__reformat_yaml(location, fmt) + + # ------------------------------------------------------------------ + # yolov8 — relative paths + # ------------------------------------------------------------------ + + def test_yolov8_paths_are_relative(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov8") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertEqual(content["test"], "test/images") + + def test_yolov8_without_test_split_removes_test_key(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_without_test(), tmp, "yolov8") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertNotIn("test", content) + + def test_yolov8_preserves_other_fields(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov8") + content = _read_data_yaml(tmp) + self.assertEqual(content["nc"], 3) + self.assertEqual(content["names"], ["cat", "dog", "bird"]) + self.assertEqual(content["roboflow"]["license"], "MIT") + + # ------------------------------------------------------------------ + # yolov9 — relative paths (was completely missing before fix) + # ------------------------------------------------------------------ + + def test_yolov9_paths_are_relative(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov9") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertEqual(content["test"], "test/images") + + def test_yolov9_without_test_split_removes_test_key(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_without_test(), tmp, "yolov9") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertNotIn("test", content) + + # ------------------------------------------------------------------ + # Legacy formats — absolute paths + # ------------------------------------------------------------------ + + def test_yolov5pytorch_uses_absolute_paths(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov5pytorch") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], os.path.join(tmp, "train/images")) + self.assertEqual(content["val"], os.path.join(tmp, "valid/images")) + self.assertEqual(content["test"], os.path.join(tmp, "test/images")) + + def test_yolov5pytorch_without_test_split_removes_test_key(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_without_test(), tmp, "yolov5pytorch") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], os.path.join(tmp, "train/images")) + self.assertEqual(content["val"], os.path.join(tmp, "valid/images")) + self.assertNotIn("test", content) + + def test_yolov7pytorch_uses_absolute_paths(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov7pytorch") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], os.path.join(tmp, "train/images")) + self.assertEqual(content["val"], os.path.join(tmp, "valid/images")) + self.assertEqual(content["test"], os.path.join(tmp, "test/images")) + + def test_mt_yolov6_uses_absolute_paths(self): + initial = dict(_INITIAL_YAML) + initial["train"] = "./train/images" + initial["val"] = "./valid/images" + initial["test"] = "./test/images" + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, initial) + self._reformat(self._version_with_test(), tmp, "mt-yolov6") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], os.path.join(tmp, "train/images")) + self.assertEqual(content["val"], os.path.join(tmp, "valid/images")) + self.assertEqual(content["test"], os.path.join(tmp, "test/images")) + + # ------------------------------------------------------------------ + # Edge cases + # ------------------------------------------------------------------ + + def test_strips_double_parent_prefix(self): + """``../../train/images`` should become ``/train/images``.""" + initial = dict(_INITIAL_YAML) + initial["train"] = "../../train/images" + initial["val"] = "../../valid/images" + initial["test"] = "../../test/images" + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, initial) + self._reformat(self._version_with_test(), tmp, "yolov5pytorch") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], os.path.join(tmp, "train/images")) + self.assertEqual(content["val"], os.path.join(tmp, "valid/images")) + + def test_non_yolo_format_not_modified(self): + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "coco") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "../train/images") + self.assertEqual(content["val"], "../valid/images") + + # ------------------------------------------------------------------ + # Independence from ultralytics (the root cause of issue #240) + # ------------------------------------------------------------------ + + def test_yolov8_works_without_ultralytics(self): + """Paths must be fixed even when ultralytics is not installed.""" + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + with patch( + "roboflow.util.versions.import_module", + side_effect=ModuleNotFoundError, + ): + self._reformat(self._version_with_test(), tmp, "yolov8") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertEqual(content["test"], "test/images") + + def test_yolov8_works_with_any_ultralytics_version(self): + """Paths must be fixed regardless of the installed ultralytics version.""" + with tempfile.TemporaryDirectory() as tmp: + _write_data_yaml(tmp, dict(_INITIAL_YAML)) + self._reformat(self._version_with_test(), tmp, "yolov8") + content = _read_data_yaml(tmp) + self.assertEqual(content["train"], "train/images") + self.assertEqual(content["val"], "valid/images") + self.assertEqual(content["test"], "test/images")