Drop the .move accessor

[niksirbi]

Description

What is this PR

• Bug fix
• Addition of a new feature
• Other: removing an existing feature

Why is this PR needed?
Closes #319, for reasons explained in that issue.
Closes #257 as part of the course.

What does this PR do?

• port functionalities we want to keep outside move_accesor.py
• delete move_accessor.py outside
  • some validations ported to save_poses.py (because they were only used upon saving anyway).
  • class variables for dim_names and var_names moved to a new movement_dataset.py module, which now defines these per dataset type
• Delete accessor tests, and modify the integration tests that used the accessor (they now use the functional syntax)
• Remove all mentions of the accessor from the Getting Started guide in docs
• Update examples to use the functional syntax instead of the accessor

Note

This was less painful than I anticipated, thanks to our tests 🎉
I just had to remove the accessor class, see which test break.
and remove or modify them accordingly.

How has this PR been tested?

Yeah, by ensuring that existing test still pass, and full coverage is maintained.
Note that the overall coverage percentage has gone down by 0.01%, because although we still have the same number of uncovered lines, the total lines of code have gone down.

Is this a breaking change?

YES: code that was using the accessor will break after the next release.

Does this PR require an update to the documentation?

Yes, and it's been updated.

Checklist:

• The code has been tested locally
• Tests have been added to cover all new functionality
• 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 (6c32608) to head (0d27de6).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main     #322      +/-   ##
==========================================
- Coverage   99.77%   99.76%   -0.02%     
==========================================
  Files          15       14       -1     
  Lines         909      846      -63     
==========================================
- Hits          907      844      -63     
  Misses          2        2              

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

[niksirbi]

Reminder:

• Rebase to main after Update integration tests #295 is merged, to adopt the linkcheck config that was modified there.

[lochhh]

Thanks @niksirbi ! My comments are mostly related to validation. I think we don't need the movement_dataset module now, as all it does is defining constants - these can go into the respective dataset validators. I also added cross-references - the kinematics functions are lengthy but should be better once #328 and #320 are closed.

[lochhh]

I think DIM_NAMES, VAR_NAMES can go into validators/datasets.py: ValidBboxesDataset, ValidPosesDataset, because this is part of our expectation of a valid Bboxes/Poses dataset.
Not sure we need this module (yet?), since users are not expected to instantiate and use instances of MovementDataset, etc. - which may be confused with the movement dataset on the website. This module would make sense if we have something like BboxesDataset.load(...)

[niksirbi]

You are right. We may need sth like this later, but not now.
Would you prefer to have DIM_NAMES, VAR_NAMES as class variables in the above validator classes, or on the validators.py module level? I guess the first makes mores sense?

[lochhh]

Agreed. First one makes more sense.

[niksirbi]

Alright, I implemented them as class variables in the respective dataset validator classes and removed the movement_dataset.py module.

Now that we have them there, I was tempted to fiddle with the validators to make the order of dimensions less hard-coded, but I stopped myself because that belong to #210 and #236.

[sonarqubecloud]

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

[niksirbi]

Thanks for the review @lochhh!. I've removed the movement_dataset.py module and moved DIM_NAMES and VAR_NAMES to class variable in their respective dataset validators.

I also removed codecov/project as an absolute merge requirement, because as this PR shows, we don't care if the overall coverage % goes down (e.g. if we remove code). codecov/patch is still required (all new diff lines must be covered).