Small edits to ValidBboxesDataset (1/4)

[sfmig]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?
Two small edits are needed in ValidBboxesDataset to continue with the bboxes work:

1. Adding frame_array to ValidBboxesDataset.
   Usually running inference on a video using a detection model will output a file (often a csv) that links each detection (aka bounding box) to a frame in the video. As a result, specific frame numbers appear in the output file (see here for a few examples). We would like to be able to pass these specific frame numbers to the bounding boxes dataset if they are defined.

2. Setting default track IDs to 1-based integers.
   This follows from the discussion at Change default names for individuals if none are passed to pose dataset #217.

What does this PR do?

1. Adds frame_array as an optional attribute to the ValidBboxesDataset.

   • If no frame numbers are provided, we assign numbers based on the first dimension of the position array, and starting from 0.
   • If frame numbers are provided, we only check that the frame_numbers value is a column vector (a numpy array of size (n,1) with n being the number of frames).

2. Sets the default IDs for the individual_names to start with 0 ('id_0', 'id_1',...).

This PR also includes:

• a minor fix in the tests that renames a bboxes dataset from poses --->ds.
• two small fixes in the test_sample_data module to adapt to the new metadata.yaml file (which includes bbox data) that is now in the GIN repo. One fix is temporary (noted with a comment) to ensure the tests pass in this PR.

Question: We do not check that the frames are monotonically increasing (i.e. sorted) or that there are no gaps - should we? I think we shouldn't, but let me know thoughts.

References

\

How has this PR been tested?

Tests pass locally and in CI.

Is this a breaking change?

No.

Does this PR require an update to the documentation?

No.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• [ n/a ] The documentation has been updated to reflect any changes
• The code has been formatted with pre-commit

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 99.71%. Comparing base (292a374) to head (7b79d17).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #230   +/-   ##
=======================================
  Coverage   99.71%   99.71%           
=======================================
  Files          13       13           
  Lines         702      714   +12     
=======================================
+ Hits          700      712   +12     
  Misses          2        2           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[niksirbi]

I have my reservations/questions about the use of frames_array here, but I will raise them in PR #229, because they have to be interpreted in conjunction with how the frame_array information is used during data loading.

But in case we keep the frame_array as is, should we also accept shape of (1, n_frames) and (n_frames,)? It's easy for users to make errors in the shape of 1D vectors and it's trivial to add a converter in the validator that will reshape it to the correct format. But this is very likely to be an overkill, so feel free to ignore.

Other than that, this PR looks good.

[niksirbi]

Some thoughts on refactoring dataset validators, to be considered for a potential future issue/PR:

I think there should be a way of defining a ValidMoveDataset base class which can validate what's shared across poses and bboxes. Then, ValidPosesDataset and ValidBBoxesDataset can inherit from the base, and only do the "extra" validations specific to each dataset type.

[sfmig]

Some thoughts on refactoring dataset validators, to be considered for a potential future issue/PR:

I think there should be a way of defining a ValidMoveDataset base class which can validate what's shared across poses and bboxes. Then, ValidPosesDataset and ValidBBoxesDataset can inherit from the base, and only do the "extra" validations specific to each dataset type.

I opened an issue #241

[sfmig]

should we also accept shape of (1, n_frames) and (n_frames,)? It's easy for users to make errors in the shape of 1D vectors and it's trivial to add a converter in the validator that will reshape it to the correct format. But this is very likely to be an overkill, so feel free to ignore.

@niksirbi I liked this converter idea, and had a go here. But I think it's best to not define a converter for the frame_array. This is because:

• it is a bit hacky, as I would like to validate the input first and then coerce it (to detect and print error if users pass a list for example), and
• it makes it a bit inconsistent relative to the other arrays. Currently we don't do such conversions for any of the other arrays (although I don't think we can).

I reverted the commit for now, but let me know if you want to discuss

[sonarqubecloud]

Quality Gate passed

Issues
1 New issue
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarCloud