wadpac / GGIRread

Functions for reading accelerometer data files
https://CRAN.R-project.org/package=GGIRread
Apache License 2.0
5 stars 3 forks source link

use last valid value as imputation value, not last value from the block being imputed #46

Closed l-k- closed 1 year ago

l-k- commented 1 year ago

@vincentvanhees sorry, I should have noticed this yesterday. Currently a faulty block is being imputed with the last value from that same block. Is that correct? Should it maybe be imputed with the last valid value, which at that point is stored in rawAccel[1]?

codecov-commenter commented 1 year ago

Codecov Report

Merging #46 (20bd799) into main (dc6466e) will not change coverage. The diff coverage is 66.66%.

:exclamation: Current head 20bd799 differs from pull request most recent head b3cacfa. Consider uploading reports for the commit b3cacfa to get more accurate results

:exclamation: Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more.

@@           Coverage Diff           @@
##             main      #46   +/-   ##
=======================================
  Coverage   84.27%   84.27%           
=======================================
  Files           9        9           
  Lines         960      960           
=======================================
  Hits          809      809           
  Misses        151      151           
Files Changed Coverage Δ
R/readAxivity.R 81.98% <66.66%> (ø)
vincentvanhees commented 1 year ago

Ok, whether the new block (rawAccel) is correct we will only know until the next block is is read as only then we will be able to check whether the block had an abnormal duration. So, maybe this is a lot harder to assess.

An alternative approach is to use one standard imputation c(0, 0, 1), and log all imputation events as we already do, and let user decide on a more appropriate imputation if they think that is relevant (edit: I mean that the user will have to do that outside the function with their own code).

l-k- commented 1 year ago

I definitely may be wrong because there's a lot going on, but I think the first element of rawAccel (rawAccel[1]) is coming from the block before prevRaw, so it's confirmed to be good.

When we get to the imputed values line, we have just read the new block (raw) and we determined that the previous block (prevRaw) is faulty. If prevRaw wasn't faulty, we would put it into rawAccel[2:rawLast,] . But rawAccel[1] has actually already been filled in by the last value of the block before prevRaw. That happened at the end of processing of the block before prevRaw, here