-
Notifications
You must be signed in to change notification settings - Fork 1.8k
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
Support DeepSparse Model Detection Results #297
Conversation
There was a problem hiding this 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.
Hi @mwitiderrick 👋🏻! Thanks for your interest in |
Hey @SkalskiP this is specific for DeepSparse detection models that will return a |
@onuralpszr I see some problems with the 88-character line length in this PR. We have a line in the docstring that can't be made shorter. |
Well I was going to fix but @mwitiderrick just did it. So all good :) @mwitiderrick thank you |
@mwitiderrick btw If you don't mind can you squash your commits to make it clean or we can handle in merge as well. |
@onuralpszr okay with the option for handling at merge time |
@mwitiderrick and @onuralpszr, but that is not valid line braking 🤣 you cant break string like that. Not to mention that example must look good in docs. @onuralpszr to be honest I care more about code example readability in docs than perfect 88-character lines in docsting.
|
@onuralpszr As for squash, I'm in favor. Let's just make sure @mwitiderrick will get the credit for contribution, and that squashed commit won't be assigned to me or other person who click "merge" button. |
Sorry I might be decide too quickly. Let me check it out properly. I will handle squash part too. |
Signed-off-by: Onuralp SEZER <thunderbirdtr@fedoraproject.org>
Is it all good guys ? :)) |
lgtm |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, thank your for your contribution. Merging in
"6a2203c" just his name and all clean. @mwitiderrick again awesome job and thank you again ! |
@onuralpszr when does the new feature become available via pip install? |
#296 (comment). that's the only answer I can find :) but that is question for @SkalskiP :) |
@onuralpszr Good enough for now ;) But I still think we need deeper into that formatting. Perfect solution would be 88 characters for code and 120 for docstrings.
@mwitiderrick 1-2 weeks is the timeline for the next release. We might do some pre-releases next week. |
Support DeepSparse Model Detection Results
https://colab.research.google.com/drive/1LsQ4xdnFfDVIrGBgmwjq-E8v_M_B-zqd?usp=sharing