childmindresearch / wristpy

https://childmindresearch.github.io/wristpy/
GNU Lesser General Public License v2.1
2 stars 1 forks source link

feat: Implementing 2023 GGIR non-wear detection algorithm #49

Closed Asanto32 closed 1 month ago

Asanto32 commented 2 months ago

Adding the detect_nonwear function to metrics.py. This function implements the 2023 GGIR non-wear detection algorithm to determine when the non-wear flag should be set to true (1) or false (0).

codecov[bot] commented 2 months ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 100.00%. Comparing base (bba82e4) to head (512940a).

Additional details and impacted files ```diff @@ Coverage Diff @@ ## development #49 +/- ## ============================================= Coverage 100.00% 100.00% ============================================= Files 4 4 Lines 98 134 +36 ============================================= + Hits 98 134 +36 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

Asanto32 commented 1 month ago

@ReinderVosDeWael Updated to address all comments and discussion from yesterday. Not sure if you want to take a final look before merging. Note that the new test_compute_nonwear_value_array is a little intricate but it is to ensure we cover all possible outcomes (0, 1, 2, 3), and the best way I thought to do that was to manually modify one data axis at a time for the "special" cases to get the 1 and 2 non-wear values as outputs.

ReinderVosDeWael commented 1 month ago

You should know that "test is a little intricate" is like a swan song to summon me :P. Yeah, if you need such a large if statement to handle your parameterization, then you should not be parameterizing your test i.e. this one needs to be broken up.

Will review in a sec.

Asanto32 commented 1 month ago

Yeah, if you need such a large if statement to handle your parameterization, then you should not be parameterizing your test i.e. this one needs to be broken up.

Makes sense. I realize now it can just be broken into individual case tests (well two cases are easily parameterized)