Provide a generic load_poses.from_file() function #107

[DhruvSkyy]

Description

What is this PR

Creates a generic function from_file() which is identical to:

• from_dlc_file()
• from_sleap_file()
• from_lp_file()

Can be called by specifying source software,

def from_file(
    file_path: Union[Path, str],
    source_software: Literal["DeepLabCut", "SLEAP", "LightningPose"],
    fps: Optional[float] = None

• Bug fix
• Addition of a new feature
• Other

References

Closes #98

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 ✅

Comparison is base (9ee30c7) 99.12% compared to head (33e09ed) 99.13%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #110   +/-   ##
=======================================
  Coverage   99.12%   99.13%           
=======================================
  Files           8        8           
  Lines         458      461    +3     
=======================================
+ Hits          454      457    +3     
  Misses          4        4           

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

[niksirbi]

Thanks for this @DhruvSkyy!

I've left a few comments, but apart from these, let's discuss testing.

When it comes to testing a function like from_file that delegates its operations to other functions (from_dlc_file, from_sleap_file, from_lp_file), which are already thoroughly tested, our primary goal should be to ensure that the delegation logic is correct without necessarily re-testing the functionality of the underlying functions in depth.

I would probably add one more function to the TestLoadPoses class, doing sth akin to the following:

    @pytest.mark.parametrize(
        "source_software",
        ["SLEAP", "DeepLabCut", "LightningPose", "Unknown"]
    )
    @pytest.mark.parametrize("fps", [None, 30, 60.0])
    def test_from_file_delegates_correctly(self, source_software, fps):
        """Test that the from_file() function delegates to the correct
        loader function according to the source_software."""

        software_to_loader = {
            "SLEAP": "movement.io.load_poses.from_sleap_file",
            "DeepLabCut": "movement.io.load_poses.from_dlc_file",
            "LightningPose": "movement.io.load_poses.from_lp_file",
        }

        if source_software == "Unknown":
            with pytest.raises(ValueError):
                load_poses.from_file("some_file", source_software)
        else:
            with patch(software_to_loader[source_software]) as mock_loader:
                load_poses.from_file("some_file", source_software, fps)
                mock_loader.assert_called_with("some_file", fps)

The above test just ensures that the correct underlying loader is called with the right parameters passed to it. We do not actually want to call the underlying functions (because we have tested that already many times), that's why I've used mocking (pretend that we are calling them).

FYI, you would have to from unittest.mock import patch and you would also need to handle the case of unknown software in the implementation of from_file (see one of my comments).

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

[niksirbi]

Thanks for implementing the suggestions @DhruvSkyy !!
I've updated the documentation accordingly. I'll approve and merge this PR now 🎉