Sage-Bionetworks / mhealthx

mhealthx is a software pipeline that automates feature extraction from mobile health data saved as a Synapse project (synapse.org).
http://sage-bionetworks.github.io/mhealthx/
Other
13 stars 11 forks source link

Calculating gait symmetry in pyGait #5

Open telferm57 opened 6 years ago

telferm57 commented 6 years ago

I ported the mhealthx code to python3. In doing so , I changed calulation of step and stride period to

step_period = int(1 // avg_step_duration) stride_period = int(1 // avg_stride_duration)

These are passed to gait_regularity_symmetry(data, step_period, stride_period) where they are used to index in to the autocorrelation coeffecients , hence have to be int.

2 issues : 1) in the 2.x code the step and stride periods are calculated : step_period = 1 / avg_step_duration stride_period = 1 / avg_stride_duration as the avg_x variables are floats, these will produce floats, so the index into the autocorrelation coefficients should fail

2) should we not be indexing into the autocorrelation matrix at number of samples that an average stride or step lenth is, not the period in seconds ?

binarybottle commented 6 years ago

Thank you for catching this, @telferm57 -- I believe that indexing into the autocorrelation matrix at number of samples would be more appropriate, as long as the matrix is constructed in a consistent manner. Please take a stab at this. And thank you for porting to python3 -- please make a pull request so everyone benefits!

telferm57 commented 6 years ago

OK, I'll fork the project and implement my changes on a fork .... then do a pull reqeust on that - is that OK? There is an issues around python 2/3 - I've only changed the code I'm using, i.e. the gait code, not the whole code base. Not sure of the best way to proceed - maybe develop in 3, then backport to 2? I haven't got the time to convert the whole codebase ...

binarybottle commented 6 years ago

That sounds great. Ideally we would want to move everything over to 3, and I tried to make the code work in both (parentheses around print statements, floats when multiplying/dividing), so it shouldn't be too difficult. You should be able to code up the changes such that they would work in both 2 and 3, no?

telferm57 commented 6 years ago

I'll make any changes 2/3 compliant ... plus I've run the the code through 2to3 factorize as per http://python-future.org/futurize.html. I'm creating a pull request now, though changes not yet implemented, so you can see my proposed changes.

telferm57 commented 6 years ago

Whoops .. I'm new to github collaborative working - please let me know if I can do things better ! btw, I don't see any tests anywhere - am I missing something ?