Skip to content
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

[BUG] YOLOv5 yaml path ignored #1903

Closed
2 of 6 tasks
victor1cea opened this issue Jun 21, 2022 · 5 comments
Closed
2 of 6 tasks

[BUG] YOLOv5 yaml path ignored #1903

victor1cea opened this issue Jun 21, 2022 · 5 comments
Labels
bug Bug fixes good first issue Good for newcomers

Comments

@victor1cea
Copy link
Contributor

System information

  • OS Platform and Distribution (e.g., Linux Ubuntu 16.04): Ubuntu 22.04
  • FiftyOne installed from (pip or source): pip
  • FiftyOne version (run fiftyone --version): 0.16.3
  • Python version: 3.9.12

Describe the problem

The 'path' from dataset.yaml in YOLOv5 datasets is ignored/not handled.
Example of dataset.yaml stored

names:
[cars]
nc: 1
train: ./images/train
val: ./images/val
test: ./images/test
path: some/path # ignored

path should be used like a base path for the splits e.g. train path would actually be some/path/images/train. The importer ignores this and the splits are not found. The exporter does not write path: ... in the exported dataset.yaml.

Code to reproduce issue

Provide a reproducible test case that is the bare minimum necessary to generate
the problem.

# read any yolov5 and have values for "path" key in dataset.yaml
yolo_dataset: fo.Dataset = fo.Dataset(name)
for split in splits:
    yolo_dataset.add_dir(
        yaml_path=yaml_path,
        dataset_type=fo.types.YOLOv5Dataset,
        split=split,
        tags=split,
        include_all_data=True
    )

# export the dataset
split_view = self.yolo_dataset.match_tags(split)
split_view.export(export_dir,
                  yaml_path=export_dir + '/dataset.yaml',
                  dataset_type=fo.types.YOLOv5Dataset,
                  label_field='ground_truth',
                  split=split,
                  include_confidence=True
                  )

Other info / logs

What areas of FiftyOne does this bug affect?

  • App: FiftyOne application issue
  • Core: Core fiftyone Python library issue
  • Server: Fiftyone server issue

Willingness to contribute

The FiftyOne Community encourages bug fix contributions. Would you or another
member of your organization be willing to contribute a fix for this bug to the
FiftyOne codebase?

  • Yes. I can contribute a fix for this bug independently.
  • Yes. I would be willing to contribute a fix for this bug with guidance
    from the FiftyOne community.
  • No. I cannot contribute a bug fix at this time.
@victor1cea victor1cea added the bug Bug fixes label Jun 21, 2022
@brimoor
Copy link
Contributor

brimoor commented Jun 23, 2022

Hi @victor1cea 👋

Feel free to make a quick PR to add support for an optional path parameter; I wasn't aware that it was a possible parameter in dataset.yaml. Do you have a link for reference?

It'd be great if you could update the link in our YOLOv5 documentation to point to a place that provides more complete documentation for dataset.yaml 🤗

https://github.com/voxel51/fiftyone/blob/develop/docs/source/user_guide/dataset_creation/datasets.rst#yolov5dataset
https://github.com/voxel51/fiftyone/blob/develop/docs/source/user_guide/export_datasets.rst#yolov5dataset

@brimoor brimoor added the good first issue Good for newcomers label Jun 23, 2022
@victor1cea
Copy link
Contributor Author

Hi @brimoor
I will gladly make a PR.
This is the reference from the official repo.
Go to 1. Create Dataset -> Or manually prepare your dataset -> 1.1 Create dataset.yaml.
They also unofficially support the optional download parameter as seen here. If you think that support for this parameter is needed, just let me know.

@brimoor
Copy link
Contributor

brimoor commented Jun 23, 2022

Cool! Supporting download is fine with me if you'd like to add it. I guess it would download the zip and unpack it to the locations specified by train, val, path, etc.?

@victor1cea
Copy link
Contributor Author

Yes, exactly. I will look into adding support for download too. 😄

brimoor added a commit that referenced this issue Aug 23, 2022
@brimoor
Copy link
Contributor

brimoor commented Aug 23, 2022

Fixed by #2016!

@brimoor brimoor closed this as completed Aug 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Bug fixes good first issue Good for newcomers
Projects
None yet
Development

No branches or pull requests

2 participants