[WIP] add frequency API and compute coffeine API #51
Conversation
coffeine/power_features.py
Outdated
| return frequency_bands | ||
|
|
||
|
|
||
| def make_coffeine_df(C: np.ndarray, |
There was a problem hiding this comment.
@apmellot @antoinecollas @apmellot @DavidSabbagh @hubertjb @bmalezieux
A small but important & overdue function :) What do you think about its naming? I think I am in principle happy with it. Other intuitive options would be to call it make_coffeine_data_frame or make_covariance_data_frame. I would be OK with making it explicit (data_frame rather than df). Slight preference for coffeine make_coffeine_data_frame for branding and energizing weirdness. I understand make_coffeine_data_frame could be more boring and predictable (in the dispassionate scientific sense), which would be not bad either.
Any thoughts? (for the bigger picture, please take minute to zoom out and look at the entire PR / the linked issues).
There was a problem hiding this comment.
I prefer make_coffeine_data_frame to be consistent with the function compute_coffeine
There was a problem hiding this comment.
well compute_coffeine is of course also up for discussion but I feel good about it actually.
I can live with make_coffeine_data_frame and make_covariance_data_frame – although I have slight preference for coffeine as what we support here is not necessarily a covariance but any SPD matrix.
|
I would go with make_covariance_data_frame or make_filter_bank_covariance
|
|
Now would be a good moment for a first round of reviews at the conceptual/strategic/API level. Once the roadmap / idea is clear I‘ll work on new unit tests. |
Also note - I put some thoughts into this with possible future developments in mind (alternative frequency schemes) and alternative feature computation options, including additional covariance estimators (for which we should best update the MNE-Python API, estimator by estimator, as this is easy, specific and gets us all benefits of MNE‘s capability of handling multiple sensor/electrode types + data rank). |
|
ok seems like we‘ve got docs 😸 |
|
… Message ID: ***@***.***>
|
|
works for me
… Message ID: ***@***.***>
|
|
ok - we've never been that far!
it feels quite big now. unless anyone protests I'd merge this to see how docs / website look. |
|
... one more test missing :) |
|
Ok, I'm merging 🚀 🎸 – if you have complaints please open an issue / PR 😸 🐙 |
Closes #41 #42 #11 #28 – API included in both notebook and in library.
This PR implements 3 main ideas:
All this should make it far easier for us to express & perform data analyses, reducing biolerplate & cognitive burden. And newcomers should be happier as well.