bugfix - downloading yolov8 files incorrectly amends data.yaml - #334
Conversation
| self.__download_zip(link, location, model_format) | ||
| self.__extract_zip(location, model_format) | ||
| self.__reformat_yaml(location, model_format) | ||
| self.__reformat_yaml(location, model_format) # TODO: is roboflow-python a place to be munging yaml files? |
There was a problem hiding this comment.
I speculate, based on 2 days of experience with this:
In the past - yes. data.yml used to contain path, providing a path to the dataset. This would be based on where the dataset is located on the system. Now, yolov8 and yolo11 do not require that, but I don't know about other models.
There was a problem hiding this comment.
This will not fix the path attribute. It will leave the train/test/val attributes untouched with the original content that was downloaded.
There's this other PR on Annotation.js that will remove that.
| content["train"] = location + content["train"].lstrip(".") | ||
| content["val"] = location + content["val"].lstrip(".") | ||
| content["test"] = location + content["test"].lstrip(".") | ||
| if format in ["yolov5pytorch", "yolov7pytorch", "yolov8", "yolov9"]: |
There was a problem hiding this comment.
We should test yolov9 as well. I thiiiiink it was yolov5-based? Could be that it needs special treatment like this.
There was a problem hiding this comment.
Yeah, let's release this and we can test that ;-)
| ) | ||
| # silently fail | ||
| pass | ||
|
|
There was a problem hiding this comment.
I'd prefer to remove rather than keep, but let me paint an alternative.
This serves 3 purposes:
- Tell user if the version is too new (e.g. known to not work, but only at this time point)
- Tell user if they're not using ultralytics that we assume they need it
- Tell the maintainer that ultralytics is required.
In the ideal world, we'd instead soften the wording, saying something akin to "When working with yolov8, roboflow works best with ultralytics<=8.2.103. You can install it with ..."
This way, the message is not exactly incorrect in the future, but still gives good info to users.
Leaving it to you to decide what we do. Removal is also fine, in my opinion 🙂
There was a problem hiding this comment.
mmm idk, IMO this is the wrong place to do this.
Theres many reasons a user might want to download a version.
It's too weird to be checking ultralytics version as part of downloading a zip file.
|
Than you very much @tonylampada! There's an initiative in making the notebooks more reliable next week - we'll test it then. |
Description
Our train notebooks like train-yolov8-object-detection-on-custom-dataset.ipynb are having to do weird yaml amending like
When you export a dataset in yolov8 format from the UI, you get a correct yaml that has
But if download it with roboflow-python you get a modified yaml like
It's not good that the train notebook needs to undo the mistake of roboflow-python
This PR fixes that mistake.
Type of change
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?
Tested locally