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

Cast category_id field value from str to int #47

Merged
merged 2 commits into from Apr 24, 2022

Conversation

dnth
Copy link
Contributor

@dnth dnth commented Apr 23, 2022

This PR casts the category_id field value from str to int data type to be consistent with the COCO data format.

I have also checked the other fields and found everything is consistent with the format outlined in https://cocodataset.org/#format-data

Closes #46

@alexheat alexheat self-requested a review April 23, 2022 14:18
@alexheat
Copy link
Contributor

Hi @dnth , thank you! But it looks like the change you made has failed one of the test cases. It is getting an errror " ValueError: cannot convert float NaN to integer"

That test is running this notebook https://github.com/pylabel-project/samples/blob/main/yolo2coco.ipynb

Could you try your change on the dataset that is used in that notebook? I think there may be some null values that are causing the error. Most of the issues that people report are due to some kind of null values in the data sets.

There are safe ways that you can convert the field to an int while leaving null values as null, and making an error.

You can see this readme for how to run the test cases before you make a pull request https://github.com/pylabel-project/pylabel/tree/dev/tests

Thanks again Dickson

@@ -694,7 +694,7 @@ def ExportToCoco(self, output_path=None, cat_id_index=None):
"iscrowd": df["ann_iscrowd"][i],
"pose": df["ann_pose"][i],
"truncated": df["ann_truncated"][i],
"category_id": df["cat_id"][i],
"category_id": int(df["cat_id"][i]),
Copy link
Contributor

Choose a reason for hiding this comment

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

This conversion throws an error if the cat_id is null.

@dnth
Copy link
Contributor Author

dnth commented Apr 24, 2022

Hi @dnth , thank you! But it looks like the change you made has failed one of the test cases. It is getting an errror " ValueError: cannot convert float NaN to integer"

That test is running this notebook https://github.com/pylabel-project/samples/blob/main/yolo2coco.ipynb

Could you try your change on the dataset that is used in that notebook? I think there may be some null values that are causing the error. Most of the issues that people report are due to some kind of null values in the data sets.

There are safe ways that you can convert the field to an int while leaving null values as null, and making an error.

You can see this readme for how to run the test cases before you make a pull request https://github.com/pylabel-project/pylabel/tree/dev/tests

Thanks again Dickson

Ahh i see.. I didn't consider that before I made the PR.. I have amended to check for na value and ran the tests. Tests are passing on my machine. Could you please check if the code is optimal?

@dnth
Copy link
Contributor Author

dnth commented Apr 24, 2022

I find that I need to install the following before I can run the test:

pip install pytest
pip install pytest-lazy-fixture
pip install nbmake

Do you think we should add it in the test readme to make it easier for first time contributors?

@alexheat alexheat merged commit 4e7728e into pylabel-project:dev Apr 24, 2022
@alexheat
Copy link
Contributor

Thank you @dnth that looks like a good fix. I have accepted the PR and release v.37 with the fix. Could you give it a try?

Also I have added some info to the tests readme on how to install the requirements https://github.com/pylabel-project/pylabel/tree/dev/tests.

And I should also make a better guide for contributors. That is on my to do list :)

@dnth
Copy link
Contributor Author

dnth commented Apr 25, 2022

Thank you @dnth that looks like a good fix. I have accepted the PR and release v.37 with the fix. Could you give it a try?

Also I have added some info to the tests readme on how to install the requirements https://github.com/pylabel-project/pylabel/tree/dev/tests.

And I should also make a better guide for contributors. That is on my to do list :)

Yay! Looks good to me!

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.

ExportToCoco() saves category_id as string instead of integer
2 participants