Adapt move accessor to bboxes datasets

[sfmig]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other

Why is this PR needed?
This PR extends the move.validate() method to bounding boxes dataset.

What does this PR do?

• Extends the validate method of the move accessor to consider a bounding boxes dataset.
• Extends the current tests of the move accessor to cover the cases with bounding boxes dataset. To do so:
  • It adds fixtures for valid/invalid bounding boxes datasets.
  • It splits the invalid_poses_dataset fixture into separate fixtures. This allows us to reusue fixtures that are valid for both types of datasets (e.g., empty_dataset).
  • It parametrizes current tests, to extend them to valid and invalid bounding boxes datasets
  • It adds a test for the move.validate() method.
• Adapts other tests to the new fixtures for invalid datasets.

I am not sure I fully understand the move accessor so feel free to suggest better implementations!

References

This came up while working on #246 - we missed the move accessor bits while implementing the bounding boxes dataset.

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.76%. Comparing base (1f219cb) to head (f8ce80b).

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #255   +/-   ##
=======================================
  Coverage   99.76%   99.76%           
=======================================
  Files          14       14           
  Lines         854      866   +12     
=======================================
+ Hits          852      864   +12     
  Misses          2        2           

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

[lochhh]

Thanks @sfmig for the great work! Just a couple of minor comments. I also refactored the validation bit and added missing 'bboxes' description in the docstrings. The other small change is with changing function, variable names to lowercase (which were previously reported as issues)

[sfmig]

thanks @lochhh! for the great points and the extra commits, super helpful 🚀

For the tests with invalid datasets that have missing dimensions / data variables, I followed your suggestion and removed the whole-string comparison - online people agreed this is best practice as it is less brittle 🌟

However, I kept the whole-string comparison for the list of missing dimensions (or data vars). This is because in the test we don't want to check if a subset of the missing dimensions is reported in the error message. Instead we want to check the full list of missing dimensions (or data vars) is exactly as expected. The easiest way is to print the list of missing dimensions in a reproducible manner, so I kept the sorted bit in for now.

Let me know if you think differently - if all good I am happy to merge

[lochhh]

Thanks again @sfmig for addressing all comments!
Just made a couple more changes:

• updated docstrings in move_accessor
• removed the wrappers around ds.rename and ds.drop_vars in conftest.py, in preference to the more familiar, one-liner xarray syntax

Think we are good to go 🚀

[sonarcloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

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

See analysis details on SonarCloud

[sfmig]

thanks @lochhh for the edits! always eagle-eyed 🚀 ✨ 👏