roboflow
Feat: Added detection metadata
Description
As per,
- #1585
implemented detection metadata and updated related methods. Please let me know if anything needs to be fixed/changed.
Type of change
- [x] New feature (non-breaking change which adds functionality)
- [x] This change requires a documentation update (probably)
How has this change been tested, please provide a testcase or example of how you tested the change?
Here's the link to the colab that tests the implemented code.
Additional Notes
In the current implementation, is_empty returns True even when a detection has a non empty metadata. When two empty detections with metadata are merged, the metadata is retained in the merge, with everything else being empty. If this is not how it is intended to work, simply adding empty_detections.metadata = self.metadata in is_empty retains metadata in the merge, however, this returns False when metadata is non empty (with everything else being empty).
Hey @LinasKo, it says the 'Welcome Workflow' check has failed. Is this the reason the review has been halted?
On it, on it. This one will take time, likely the whole week. It can have consequences across the repo, so I must be very careful.
I've made some comments. I like your changes so far!
It shows you've thought through the problem, in places like introducing of optional variable in empty, while avoiding the pitfall of incorrectly defaulting to {}.
Hey @LinasKo, I have made the required changes. Please let me know about further changes/fixes. Thanks :)
That looks good.
For each contribution moving forward, the next step is to create a Colab notebook to test the changes. This should include generating Detections with and without metadata to demonstrate how it works, and testing with various values. It’s a bit simpler than unit testing, and it also serves as a reference for future regression testing.
Got any time for that, @souhhmm? There's a Starter Template you may use 😉
I had linked a colab in the og PR, linking once again. But now that I have changed some code, I'll re-run it. Any particular tests you'd like me to include that I might not have included @LinasKo?
Edit: I've re-ran the testcases.
Hi @souhhmm 👋
My apologies, I missed your Colab. That is indeed an amazing set of tests. I'll take over from this point - there's a few checks I need to do before I merge this.
Thank you for a great contribution!
Thank you for a fantastic contribution @souhhmm!
Most take me hours to verify, but with your test suite and thoughtful changes, I barely had to do anything. Expect to see it in the next supervision release at the end of this week!