returned empty slice of detection instead of new empty detection object (bug fix)

[MoHoss007]

This keeps the keys of the data field the same

Description

Currently when there are no tracks an empty detections object is returned which doesn't have the keys of data field that were in the original detections field, with this change those keys are also kept.

List any dependencies that are required for this change.

N/A

Please delete options that are not relevant.

• Bug fix (non-breaking change which fixes an issue)

How has this change been tested, please provide a testcase or example of how you tested the change?

It was tested on my local branch now we have

def test_keys_of_data_field(tracker, detection):
    """
    Test to verify that the keys of the .data field of the detection 
    and detection_with_tracking objects are the same.
    """
    detection_with_tracking = tracker.update_with_detections(detection)
    
    detection_keys = detection.data.keys()
    detection_with_tracking_keys = detection_with_tracking.data.keys()
    
    assert detection_keys == detection_with_tracking_keys, (
        f"Keys do not match: Detection keys: {detection_keys}, "
        f"Detection with tracking keys: {detection_with_tracking_keys}"
    )

YOUR_ANSWER

Any specific deployment considerations

N/A

Docs

N/A

[LinasKo]

Hi @MoHoss007 👋

An interesting approach and something we've not thought about - indexing by an empty list. I need to think about this.

It is, however, problematic. We would like to support Detections.empty as a special value that any of our methods recognise, as in many cases it's impossible to provide a detections instance to get the keys from.

I am curious: what does this change help solve? Does it become easier to call the data dict, knowing you'd always get the value? Would you mind sharing a small part of what you're working on?

[MoHoss007]

Hi @LinasKo , basically, I have a pipeline in which I want to keep the same data structure for detections and tracking, so before the object enters the tracking node I do some processing on it and the results are saved in the data field, it will be easier/cleaner that the keys always remain there as it will be consistent with the case where we have tracks and return a slice of detections object and the keys are there.

[LinasKo]

Thank you - this is very useful for me to hear.

[MoHoss007]

Hey @LinasKo,
Thanks for the feedback! I'd love to contribute and help solve this issue. How can I get involved?

[SkalskiP]

Hi @LinasKo and @MoHoss007 👋🏻 I'm super curious what do you think about this @LinasKo. To me it looks really reasonable.

[LinasKo]

@SkalskiP, here's a half baked opinion; what my intuition says so far. We can discuss later.

1. This introduces a new standard that competes with Detections.empty, but does not completely replace it. The problematic case is intermediary functions like Detections.merge. They may get input [] rather then [detections_1, detections_2, ...]. In the first case they still need to return Detections.empty().

2. Now there's 2 standards, and both us and contributors need to know which one to use. We also need to modify is_empty, but that's probably okay.

3. I like that we keep the keys. This is kinda like Detections.empty_like. (Not a proposal for a name - just thinking abstractly)

But, rephrasing paragraph 1, empty_like would need a detections instance. Where we don't have an instance, this proposal wouldn't work and we still need Detections.empty.

4. This seems simple, but the member access path is pretty complicated. Starting with an empty list, it goes into our getitem, through the if statement, accesses detection members and data with different logic, uses numpy. Way more complicated than Detections.empty.

5. I had one more; will update if I remember.

Despite all that - maybe it's still an improvement. Need to find some space to think.

[LinasKo]

@MoHoss007, your willingness to help is much appreciated! Especially after presenting a new path of solving it.

But issues related to Detections.empty and Detections.merge are deceptively complicated (see #943) so I'd strongly prefer if the core members of the team worked on these.

My deepest thanks for making us aware of this new approach though. And if you're willing to help us out with something else, I'd happily ping you when we have some features scoped out.

[MoHoss007]

@LinasKo I see, the points you mentioned are valid, thank you. I'm always happy to help, that would be great!

[LinasKo]

Hi @MoHoss007,

Closing this for now, however, I've added it to the list of fixes for the future.

Thanks again for spotting an alternative solution!