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

Remove class list dependency for roboflow to detection conversion #399

Merged
merged 7 commits into from
Oct 4, 2023

Conversation

akashAD98
Copy link
Contributor

remoed class_list arg from core.py file

Description

No need to ask user for providing class_list
simplify the sv.Detections.from_roboflow API and remove extra argument.

solved this issue : #394

Please delete options that are not relevant.

  • [ x] New feature (non-breaking change which adds functionality)
  • [ x] This change requires a documentation update

Docs

  • Docs updated? What were the changes:
  • once this is perfect ill update docs

remoed class_list arg from core.py file
@CLAassistant
Copy link

CLAassistant commented Oct 3, 2023

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hello there, thank you for opening an PR ! 🙏🏻 The team was notified and they will get back to you asap.

@hardikdava hardikdava changed the title Update core.py Remove class list dependency for roboflow to detection conversion Oct 3, 2023
@hardikdava
Copy link
Collaborator

Hi @akashAD98 👋 , thank you for contributing to Supervision. I would suggest you to following things first.

  • Accept and sign CLA.
  • Please read carefully about contributing guideline here
  • Attach minimal python code or colab notebook to reproduce your results or bugs.

@hardikdava
Copy link
Collaborator

@SkalskiP @capjamesg This might generate conflicts in roboflow server. Please have a look.

@hardikdava
Copy link
Collaborator

@akashAD98 I have fixed class id conversion bug.

@hardikdava
Copy link
Collaborator

@SkalskiP If we want to accept this PR then please fix the tests.

@SkalskiP SkalskiP linked an issue Oct 3, 2023 that may be closed by this pull request
@SkalskiP SkalskiP added the hacktoberfest Open for contributions during the annual Hacktoberfest event, aimed at encouraging open-source parti label Oct 3, 2023
@SkalskiP SkalskiP self-requested a review October 3, 2023 17:52
@SkalskiP
Copy link
Collaborator

SkalskiP commented Oct 3, 2023

Hi, @akashAD98 👋🏻! Could you accept the CLA?

I'll take care of tests once you do it.

@akashAD98
Copy link
Contributor Author

@hardikdava i accepted the CLA

Updated the prediction parsing logic in supervision/detection/utils.py and supervision/detection/core.py to use 'class_id' instead of 'class' names. Adapted tests in test/detection/test_utils.py accordingly to reflect the same change.
@SkalskiP
Copy link
Collaborator

SkalskiP commented Oct 4, 2023

@akashAD98 and @hardikdava, I fixed the tests and tested it on example roboflow/inference result, and everything looks fine. We can merge!

@SkalskiP SkalskiP added this to the version: 0.15.0 milestone Oct 4, 2023
@SkalskiP SkalskiP added version: 0.15.0 Feature to be added in `0.15.0` release hacktoberfest-accepted Contribute to the notion of open-source this October! labels Oct 4, 2023
@SkalskiP SkalskiP merged commit 67fa66c into roboflow:develop Oct 4, 2023
6 checks passed
@akashAD98
Copy link
Contributor Author

thanks @SkalskiP

@akashAD98 akashAD98 deleted the update_detection_sv branch October 4, 2023 12:07
@SkalskiP
Copy link
Collaborator

SkalskiP commented Oct 4, 2023

@akashAD98, thanks for help!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest Open for contributions during the annual Hacktoberfest event, aimed at encouraging open-source parti hacktoberfest-accepted Contribute to the notion of open-source this October! version: 0.15.0 Feature to be added in `0.15.0` release
Projects
Status: Current Release: Done
Development

Successfully merging this pull request may close these issues.

Update sv.Detections.from_roboflow and remove class_list requirement
4 participants