Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Tremor weekly aggregation #90

Merged
merged 13 commits into from
Dec 23, 2024
Merged

Tremor weekly aggregation #90

merged 13 commits into from
Dec 23, 2024

Conversation

nienketimmermans
Copy link
Contributor

@nienketimmermans nienketimmermans commented Dec 17, 2024

I modified the aggregation step so that the daytime hours and valid days are not taken into account.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Execution counts in the notebook.

nr_windows_daytime = df_filtered.shape[0] # number of windows during daytime on valid days
df_filtered = df_filtered.loc[df_filtered.low_freq_power <= config.movement_threshold]
nr_windows_daytime_rest = df_filtered.shape[0] # number of windows during daytime on valid days without non-tremor arm movement
# remove windows with detected non-tremor movements
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could specify that above the movement threshold, you expect there to be movement that interferes(?) with the tremor quantification. I'm not sure about the specifics, but it might make sense to clarify this subsetting.

"nr_valid_days": 1,
"nr_windows_daytime": 182,
"nr_windows_daytime_rest": 89
"nr_windows_rest": 89
},
"weekly_tremor_measures": {
"tremor_time": 16.853932584269664,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Perhaps you could specify the units here.

Copy link
Contributor

@Erikpostt Erikpostt left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking good! I made a few suggestions to change, feel free to make these changes if you agree, and then you can merge it yourself.

@nienketimmermans back to you.

@nienketimmermans nienketimmermans merged commit c286d02 into main Dec 23, 2024
1 check passed
@nienketimmermans nienketimmermans deleted the tremor_weekly_aggregation branch December 23, 2024 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants