-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
docs(notebooks): 📝 json and csv sink cookbooks are added #975
Conversation
Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
@SkalskiP do you want to merge/review this? |
Hi @onuralpszr 👋🏻, thanks for the reminder.
|
@SkalskiP thank you for tips let me update all. |
Pleasure 💜 |
@SkalskiP I updated both collabs in "the links" could you check, after your confirm I will commit all |
@SkalskiP friendly reminder ping :) |
Hi @onuralpszr , we spoke with Piotr and I'll be reviewing this one |
Hi @onuralpszr, Here's my review comments:
Sinks allow users to serialize data into formats suitable for sharing with others or storage in the filesystem. We should show the deserialization case too!
Seems like a lot of changes - apologies for the hassle! However, we do wish to equip the devs with the exact tools they need to get their problems solved 😉 Let me know if you have any questions! |
@LinasKo, thanks for putting the list together! 🙏🏻 |
Hey @onuralpszr, How's it going? Got any updates for this one? I know you're pretty busy so no rush 🙂 |
Check JSON for development in URL, for update html in commit I will do as last, Don't worry I know that problem ;) I will also remove "loading bar" I am very well aware that problem in the beginning ;) I added docs and changes most of it. I wrote complete convert from json to detections check only json please, I can do docs changes and others on CSV afterwards (%99 percent same) |
Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
@LinasKo I updated both colabs did some changes please check when you had time, thank you. |
Will do, thank you @onuralpszr ! |
Hi @onuralpszr, Thank you very much for the adjustments! It looks much better now, and I think as a user I'd be able to find what I need. I still have a few requests:
"""
"""
"""
I believe that's everything. Thanks again of the awesome work that you do! |
@LinasKo could you explain 6 again ? I think you meant "json" one for using nothing extra like "pandas" ? |
Certainly. I see now, CSV example does use
For the JSON example, I am torn - I'm not sure if we should try using |
Json doesn't use any pandas at all it just numpy and numpy is already come with sv :) I am confused Where is pandas ? it is "np" not "pd" :) |
Let me clean up bit more okay, but I am kinda close to decide leave this PR as it is and open a new one and add actual "parse json/csv to sv.Detections" and add those into notebook :) |
Yeah, I know it feels like we ought to have it, right? 🙂 Let's not be too hasty though. Let's see whether it's needed first. |
Hey @onuralpszr, Just a quick note - we're thinking of releasing |
I'll get in done. (I have some work to finish) |
I also see some requests about convert back to sv.Det.. . We may re consider. :) |
Hey @onuralpszr, How are these going? We spoke with @SkalskiP, and the plan remains the same - to focus on the cookbooks, without the A very large part of such integration would involve testing, making a colab, spending extra time verifying the changes, whereas the Colabs here seem very close to being done. |
Hi @onuralpszr, We're back after a short break. Any updates for us, by any chance? 😉 For reference, here's the changes remaining: |
Sorry for late reply and answer. Let me get it done and I also want to introduce a new feature you gonna like hopefully too. :) |
What is the best way to output the result as a video not a single frame? |
Check out VideoSink in https://supervision.roboflow.com/utils/video/#videosink |
Hey @onuralpszr This has been stuck for a little bit too long. Let's do the minimal changes necessary to merge it. I can help or take over if needed. I'm very worried that adding methods to both serialize and deserialize to CSV/JSON will expand the testing time, need Piotr's opinion, etc. I'd rather get For reference, I believe here's the outstanding changes:
|
@LinasKo you are correct, for to-do list am I going to remove "pandas" in csv example and add "null check" part as you said early. Plus I did most of the changes already about names config, bytetrack. |
@LinasKo I will update you on CSV one with null check parts after you okay with it. I will add into here and let's merge it. AND I will be sure it loads fine in docs as well (like removal of tqdm bar for prevent docs etc.) |
Thank you very much! I really appreciate it. |
@LinasKo I removed pandas import way in CSV and using "import csv" way of showing it. Please check csv one For null check we already have in place in core CSV and JSON Sinks parts, we may skip adding another layer, it will throw out error anyway.
|
Both JSON and CSV now have the The validators only check that they're arrays, but our codebase strongly relies on the contents being not-null. The whole array can be None, e.g. So, lines like that are incorrect: P.S. Looking at it again, I see my proposal to use Regarding the use of |
@LinasKo cleaner ? :) import csv
import numpy as np
import sv
def csv_to_detections(csv_file: str) -> sv.Detections:
xyxy = []
class_id = []
confidence = []
tracker_id = []
frame_number = []
class_name = []
with open(csv_file, 'r') as f:
reader = csv.DictReader(f)
for row in reader:
if row["class_id"] != "" or row["x_min"] != "" or row["y_min"] != "" or row["x_max"] != "" or row["y_max"] != "":
xyxy.append([float(row["x_min"]), float(row["y_min"]), float(row["x_max"]), float(row["y_max"])])
class_id.append(int(row["class_id"]))
confidence.append(float(row["confidence"]))
tracker_id.append(int(row["tracker_id"]))
frame_number.append(int(row["frame_number"]))
class_name.append(row["class_name"])
custom_data = {"frame_number": np.array(frame_number), "class_name": np.array(class_name)}
return sv.Detections(xyxy=np.array(xyxy), class_id=np.array(class_id), confidence=np.array(confidence), tracker_id=np.array(tracker_id), data=custom_data) |
Hey, much cleaner! :) I suggest this if we're going this way:
I can't recall which, maybe Paligemma, but we have some models that don't return a 2 issues that I see:
|
…ocs version Signed-off-by: Onuralp SEZER <thunderbirdtr@gmail.com>
484ccc4
to
0df56b1
Compare
@LinasKo all complete and table of content also fixed for both. Docs also render correctly as well. Ready to merge |
Excellent, thank you! I'll try to review it as soon as I can. |
You're welcome |
* 1 sv.Detections per frame
971214b
to
4d12853
Compare
Happy to merge this. @onuralpszr, wanna have the last look? |
Sure let me check |
Description
New JSON and CSV sink cookbooks are added. For Json it needs a version bump, I used the direct
develop
branch while preparing it.JSON collab URL: https://colab.research.google.com/drive/19aqX0QwKXxRk4sYtdEmghO2rzEAKMciM?usp=sharing
CSV collab URL: https://colab.research.google.com/drive/1-qetEcyzrBCOCq-TfVVT8AgUG3rJUhT7?usp=sharing