Time detection in g.part5 #52
Conversation
|
I forgot to mention, changes are marked as #Change 1 and #Change 2 in the text (lines 166 and 296 of the g.part5 code). |
Codecov Report
@@ Coverage Diff @@
## master #52 +/- ##
==========================================
+ Coverage 56.62% 56.68% +0.05%
==========================================
Files 53 53
Lines 7715 7727 +12
==========================================
+ Hits 4369 4380 +11
- Misses 3346 3347 +1
Continue to review full report at Codecov.
|
|
Sorry Vincent, I removed all the comments in the function in this pull request, I am going to close it and I will send it back to you again without modifying your comments in teh function. |
|
Changes in lines 171 and 283 of the code |
|
I am not able to reproduce the issue in a GENEActiv file which i reprocessed with the latest version all the way from part1 to part5. Are you doing this too or are you working with part 1 output that was generated with an earlier version of GGIR? |
|
If this is not picked up by our unit tests then we will need to create a test for it to alarm us when this happens in the future. Were you able to observe any patterns in the incorrect timestamps, e.g. where they all midnight or were they all X hours off from the actual time? |
|
This should be the problem, I was using milestone data generated with earlier versions of GGIR. I am thinking in a test, it could be somehing like: |
|
Hi Jairo, yes feel free to do that. By unit testing I was referring to the testthat part of the package . It tests the package at a function level, e.g. if function A takes X as its input does it produce the expected Y. This requires more work than adding a few lines, because probably we want to write tests that verify whether timestamps from formats around the world are handled correctly. Next, Travis then always checks for every new commit to the repository that those tests pass. It is considered an important part of quality assurance. So, having good tests is critical for this to be meaningful. For now it may be easier to just do what you suggested and add an issue that we need to build more tests for timestamps and GGIR version compatibility. |
|
Great Vincent, I have added the conditional I told you as a provisional solution. I am going to create the issue to add the test about timestamps and I will try to figure how these tests work out, and then I will go for the code for this test. |
Time detection in g.part5
Time detection in g.part5
Time detection in g.part5
Hi Vincent!
As I told you, I am active on GGIR again. The past week I found something strange, so maybe it is better if you have a look on this changes before merging them.
When running g.part5 in my computer, it didn't detect the time correctly, I mean, day and night identification had no sense at all, with larger values on mean night accelerations than in day accelerations for some days.
I used debug() to look deeply into the code and I found that when using timebb to find a determined timestamp (when looking for sustained inactivity behaviors and when looking for day and night identification), the timestamp identified did not correspond with the timestamp the code was looking for. I assumed a misfunctioning due to the class of the objects, so I as.character() to change the class of timebb and it seems to work correctly now.
As I said, this was not happening before summer, so it is strange this error, please, confirm that the error is not happening only in my data (for any reason) before to merge these changes.
Cheers,
Jairo