Thank you, @brukew, for the updates. I have a few suggestions and points to address based on our previous discussions:

1. Reorganize Code Structure
   Please re-organize your code by separating data structures from functionalities (what we refer to as tasks in Senselab).

   • The skeleton should be treated as a data structure. It should define the skeleton and optionally include utilities for visualization. Alternatively, visualization can be handled as a standalone task.
   • Pose estimation should be a task (generating the skeleton as an output). All human pose estimation models (e.g., MediaPipe, AlphaPose) should conform to a consistent data structure.
   • To encourage generalizability, I suggest integrating a second pose estimation tool, such as YOLO due to its simplicity.

2. Model Inclusion
   The current approach of including a model within the source code (e.g., src/senselab/video/tasks/pose_estimation/models/pose_landmarker.task) makes the package unnecessarily heavy. Instead, please ensure models are downloaded as needed. You can take inspiration from this example.

3. Documentation
   Please add a dedicated documentation page:

   • Explain human pose estimation as a task, its purpose, and supported models.
   • For instance, you can reference this documentation for MediaPipe.
   • Feel free to draw inspiration from the existing audio task documentation (though some sections are incomplete).

4. Tutorial
   Create a Jupyter Notebook tutorial to demonstrate:

   • How to use the interface.
   • What functionalities are available.
     Add this under a video folder in the tutorial/ directory.

5. Failing Tests
   I noticed two tests are failing:

   • test_valid_image_single_person
     • AssertionError: "Input and output image shapes should match."
   • test_visualization_single_person
     • ValueError: "Input image must contain three-channel BGR data."
       Please double-check these tests to ensure they pass.

Nice, thank you for the feedback @fabiocat93. I will address your comments and ask questions as I go.

hi @brukew , did you have any time to work further on this?

hey @fabiocat93, slowed down on development towards the end of the semester. I will continue this month.

@fabiocat93 lmk what you think about having separate estimator classes for different models. also, I kept the landmark names as they are originally listed but wondering if I should homogenize them at all.

@fabiocat93 lmk what you think about having separate estimator classes for different models. also, I kept the landmark names as they are originally listed but wondering if I should homogenize them at all.

Good question. You can have multiple estimators, each being a child of the same abstract class and producing a skeleton following the same universal structure

addressed model download by downloading models to video/tasks/pose_estimation/models upon use. Let me know if there is a better alternative to this that is more in line with senselab use.

also wondering how I can ensure tutorial works (and cells are run in tutorial) if changes haven't been pushed to main branch @fabiocat93. should I wait until after?

addressed model download by downloading models to video/tasks/pose_estimation/models upon use. Let me know if there is a better alternative to this that is more in line with senselab use.

Good for now. But you raised a good point. We will need to implement a customizable cache folder for the package (of course with a default value)

also wondering how I can ensure tutorial works (and cells are run in tutorial) if changes haven't been pushed to main branch @fabiocat93. should I wait until after?

When testing the tutorial locally, you can install the package directly from a specific branch instead of relying on pip install senselab. This allows you to ensure the tutorial runs correctly with the latest changes before they are merged into the main branch

Codecov Report

Attention: Patch coverage is 93.67089% with 20 lines in your changes missing coverage. Please review.

Project coverage is 65.46%. Comparing base (113721a) to head (8458609).
Report is 88 commits behind head on main.

@@            Coverage Diff             @@
##             main     #203      +/-   ##
==========================================
+ Coverage   60.24%   65.46%   +5.21%     
==========================================
  Files         113      128      +15     
  Lines        4017     4572     +555     
==========================================
+ Hits         2420     2993     +573     
+ Misses       1597     1579      -18     

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

A quick note for the entire group about installation: the number of dependencies, especially for video, makes installation quite heavy. Moving forward, we might want to split the package into modules like senselab[audio], senselab[video], and senselab[text]. This would simplify installation based on the user’s needs

Hey @fabiocat93, thanks for the comments - I will make all changes before merging. Here are my thoughts:

1. Yes, that makes sense. I will do that.

2. I think it gives us greater power for anything we want to do later. For example, if a video has different people over time, we could combine face analysis and pose estimation to aggregate the poses of an individual across frames which would be easier with IndividualPose object (with tweaks). It's also just intuitive to have a separate object for each individual detected. I could take it out for now though.

3. I can do this. My thinking was that users could have access to whatever the given model output is and they can work with it how they want to, but I figure that defeats the point of having these in Senselab. So I guess I'll make a Senselab keypoint mapping and attempt to not lose any information across the models - so it includes all possible key points across models. What do you think about that?

I think it gives us greater power for anything we want to do later. For example, if a video has different people over time, we could combine face analysis and pose estimation to aggregate the poses of an individual across frames which would be easier with IndividualPose object (with tweaks). It's also just intuitive to have a separate object for each individual detected. I could take it out for now though.

You convinced me with this. but how about always returning back an ImagePose object (which may include as many IndividualPose objects as the number of people detected?)

I can do this. My thinking was that users could have access to whatever the given model output is and they can work with it how they want to, but I figure that defeats the point of having these in Senselab. So I guess I'll make a Senselab keypoint mapping and attempt to not lose any information across the models - so it includes all possible key points across models. What do you think about that?

Sounds great! And please let me know if you need or want to brainstorm it further before implementing it.

@fabiocat93 Addressed the visualization and keypoint mapping comments. The unit tests kept failing on Git Hub when a plot was created - my workaround was adding a bool parameter for plotting but let me know if there is another way that you want me to handle that.