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

New function [CSVSink] - allowing to serialise Detections to a CSV file #818

Merged

Conversation

AdonaiVera
Copy link
Contributor

Description

This PR introduces the CSVSink class to address structured logging of object detection data into CSV format, as requested in issue #746. The class supports logging of bounding boxes, class IDs, confidence scores, and frame numbers, enhancing post-processing data analysis and interpretability.

The PR includes:

  • CSVSink class implementation.
  • Comprehensive unit tests for the class.
  • A demo showcasing the integration of CSVSink with a video processing pipeline.

Note: The CSVSink class was placed directly in the utils folder in the file.py file due to its capability to handle various types of file operations integral to the project's core functionality. Open to suggestions if there's a more appropriate location within the project structure.

Type of Change

  • New feature: Added CSVSink class for structured logging of detection data.

Testing

The CSVSink class was validated through a series of unit tests, covering:

  • Logging with various custom data scenarios.
  • Handling of scenarios with no detections.
  • Verification of CSV output format and data integrity.

A demo script demonstrates the class's usage within a video processing workflow, annotating video frames and logging detections to a CSV file. Demo Here

Documentation Updates

  • Added CSVSink class documentation within the module, detailing its purpose, usage, and example usage.
  • Updated README to include a section on structured detection data logging, introducing the CSVSink class.

supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
supervision/utils/file.py Outdated Show resolved Hide resolved
@SkalskiP
Copy link
Collaborator

Hi @AdonaiVera 👋🏻 ! Good start. I left a few comments. Let me know if you have any questions. I didn't look at unit tests yet. Let's work on logic first.

@AdonaiVera AdonaiVera force-pushed the allowing_serialise_detections_csv branch from 360fd62 to 1256833 Compare January 31, 2024 16:41
@AdonaiVera
Copy link
Contributor Author

Hi @SkalskiP

Thank you for your valuable feedback. I have already implemented the changes and created a PR. Additionally, I updated the demo and moved the unit test to the test/detections folder. Please let me know your thoughts on the changes and if you have any further suggestions 🥷

@SkalskiP
Copy link
Collaborator

SkalskiP commented Feb 1, 2024

As I already mentioned. For the method/function to be visible in docs. It needs to be public, and it needs to have docstring. CSVSink methods are undocumented, and as a result, users don't see them in docs.

I recommend using mkdocs serve locally to preview your docs changes.

Screenshot 2024-02-01 at 13 26 00

docs/detection/tools/csv_sink.md Outdated Show resolved Hide resolved
mkdocs.yml Outdated Show resolved Hide resolved
supervision/detection/tools/csv_sink.py Outdated Show resolved Hide resolved
supervision/detection/tools/csv_sink.py Outdated Show resolved Hide resolved
supervision/detection/tools/csv_sink.py Outdated Show resolved Hide resolved
supervision/detection/tools/csv_sink.py Show resolved Hide resolved
supervision/detection/tools/csv_sink.py Outdated Show resolved Hide resolved
test/detection/test_csv.py Outdated Show resolved Hide resolved
test/detection/test_csv.py Outdated Show resolved Hide resolved
test/detection/test_csv.py Outdated Show resolved Hide resolved
@AdonaiVera
Copy link
Contributor Author

Hi @SkalskiP 👋
I have made all the necessary changes. However, there are still some questions that I left on the thread.

I tried to handle multiple scenarios, but I can work with more if needed. But before that, I need to solve the question about what happens if the custom data changes between detections.

Thanks 🚀

@SkalskiP
Copy link
Collaborator

SkalskiP commented Feb 2, 2024

I made a few docs updates in the meantime. Nothing important. There was no point in forcing you to make corrections. These were very minor things.

@SkalskiP
Copy link
Collaborator

SkalskiP commented Feb 2, 2024

Hi @AdonaiVera 👋🏻 I made two small commits to your branch today. I really wanted to close it out before the weekend. Feel free to ask questions here in comments.

Overal awesome work! Thanks a lot for your help! 🔥

@SkalskiP SkalskiP merged commit 87a4927 into roboflow:develop Feb 2, 2024
8 checks passed
@AdonaiVera
Copy link
Contributor Author

Thank you! We did an amazing collaborative work. @SkalskiP 💪

Now that I have the full perspective, I'm ready to create the JSON saving function, I'll update that PR soon 🚀

@SkalskiP
Copy link
Collaborator

SkalskiP commented Feb 2, 2024

Awesome @AdonaiVera! Cant wait to see that! 🔥

@AdonaiVera AdonaiVera deleted the allowing_serialise_detections_csv branch February 9, 2024 04:20
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.

None yet

2 participants