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

feature: load detection class names from yaml file #24

Merged
merged 9 commits into from
Dec 29, 2021

Conversation

phinik
Copy link
Member

@phinik phinik commented Dec 14, 2021

  • Added the possibility to load (detection) class names from a respective yaml file with key 'detection'

Proposed changes

added the possibility to load (detection) class names from a yaml file (key 'detection')

Necessary checks

  • Update poetry package version semantically
  • Write documentation
  • Create issues for future work
  • Test on your machine

	- added the possibility to load (detection) class names from a respective yaml file with key 'detection'
@phinik phinik changed the title feature: feature: load detection class names from yaml file Dec 14, 2021
@Flova Flova self-requested a review December 17, 2021 11:10
yoeo/utils/utils.py Outdated Show resolved Hide resolved
	- dropped support for non-yaml class files
	- replaced data/TORSO.names with data/yoeo_names.yaml
Copy link
Member

@Flova Flova left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some minor comments. Otherwise it is fine imo.

yoeo/detect.py Outdated Show resolved Hide resolved
yoeo/test.py Outdated Show resolved Hide resolved
yoeo/train.py Outdated Show resolved Hide resolved
yoeo/utils/utils.py Outdated Show resolved Hide resolved
@Flova Flova self-requested a review December 22, 2021 13:37
@Flova
Copy link
Member

Flova commented Dec 22, 2021

One could also add the segmentation names to the printout in the evaluation:
This would be a bit more effort, but it would be nice now that they are available.

Copy link
Member

@jaagut jaagut left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @phinik for your nice contribution!

yoeo/detect.py Outdated Show resolved Hide resolved
yoeo/utils/utils.py Outdated Show resolved Hide resolved
@jaagut
Copy link
Member

jaagut commented Dec 22, 2021

We probably also want to adapt the path in

names=data/261.names
to the new format.

Before we do that, I would suggest moving the TORSO.names / yoeo_name.yaml file from the data dir to the config dir?
Then, we would need to adapt the path also in the detect.py...

What is your opinion?

@phinik
Copy link
Member Author

phinik commented Dec 22, 2021

We probably also want to adapt the path in

names=data/261.names

to the new format.
Before we do that, I would suggest moving the TORSO.names / yoeo_name.yaml file from the data dir to the config dir? Then, we would need to adapt the path also in the detect.py...

What is your opinion?

I leave it up to you and Flova to decide on this.

@Flova
Copy link
Member

Flova commented Dec 23, 2021

We probably also want to adapt the path in

names=data/261.names

to the new format.

But the 261.names is not in the repo and I wonder why the 261.data file is in the config folder xD
The dataset was just used for local tests.

Before we do that, I would suggest moving the TORSO.names / yoeo_name.yaml file from the data dir to the config dir? Then, we would need to adapt the path also in the detect.py...

I also wonder a bit if we really want to do this, as the .data, as well as the .names file, are normally part of the dataset. But we should adapt the custom.data example file to the yaml version never the less.

We also need to adapt the README.md section that describes the expected data structure. Especially this section: https://github.com/bit-bots/YOEO/tree/main#classes
A sample yaml for TORSO could be added.

@jaagut
Copy link
Member

jaagut commented Dec 29, 2021

We also need to adapt the README.md section that describes the expected data structure. Especially this section: https://github.com/bit-bots/YOEO/tree/main#classes
A sample yaml for TORSO could be added.

This should be another PR

@jaagut jaagut mentioned this pull request Dec 29, 2021
@jaagut jaagut mentioned this pull request Dec 29, 2021
4 tasks
@jaagut jaagut self-requested a review December 29, 2021 15:50
@jaagut jaagut merged commit 3c36357 into main Dec 29, 2021
@jaagut jaagut deleted the feature/class_names_from_yaml branch December 29, 2021 16:13
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants