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

Integrate bedtime and wakeup time statistics in get_sleep_statistics #65

Open
Vaeliss opened this issue Dec 22, 2023 · 3 comments
Open
Assignees
Labels
enhancement New feature or request

Comments

@Vaeliss
Copy link
Collaborator

Vaeliss commented Dec 22, 2023

At the moment bedtime and wake-up time are not returned in the general pywearable.sleep.get_sleep_statistics.
They should be added.

In order to do so a sleep._compute_sleep_timestamp must be added, which will require a sleep_summary parameter as other compute functions.
The main difference to take into account for the calculation of statistics is that the parameter kind can't be applied directly as it is the case for other sleep statistics but must be mapped to functions in the utils module such as utils.get_earliest_bedtime etc..

This mapping could be done by creating a dictionary (kind_mapping) for the mapping which includes only the sleep metrics which have a different behavior w.r.t. the kind parameter. When kind needs to be applied for the calculation of sleep_statistics it should be simply proceeded by something like kind = kind_mapping.get(sleep_metric, kind) so that for metrics which require a different treatment, such as bedtime and wake-up time, the proper function is used.

@Vaeliss Vaeliss added the enhancement New feature or request label Dec 22, 2023
@Vaeliss
Copy link
Collaborator Author

Vaeliss commented Dec 22, 2023

Clearly, in such case the function sleep.get_sleep_timestamps should be uniformed to the other to call simply sleep.get_sleep_statistic and delegate the actual computation to the new function sleep._compute_sleep_timestamps

@dado93
Copy link
Owner

dado93 commented Aug 15, 2024

@Vaeliss at the moment we have:

  • get_bedtime
  • get_wakeup_time
    that work with the same principle of all the other sleep statistics.

Do you think that the function get_sleep_timestamps, which is the only one that is different from all the others, should still be left in the module? As it does not offer any of the new features (kind, return_df, and so on), I see two options here:

  • remove it
  • make it uniform to all other functions

@Vaeliss
Copy link
Collaborator Author

Vaeliss commented Aug 16, 2024

I'd probably remove it, as you said now that we have get_bedtime and get_wakeup_time, it's redundant.

However, it's used in several other modules for now (cardiac for hrv stats, stress module, and the labfront loader), so we need to remove it and at the same time substitute it with the two funcs above in those cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants