Issues in Ex 12.2

[jstac]

trafficstars

Mismatch between question (a) and solution to question (a) both in phrasing and answer?

[smoia]

This is ready for review (but also very simple)! @physiopy/peakdet

[62442katieb]

Previous comment was outdated, that's my bad. LGTM!

[smoia]

Good catch @62442katieb ! You were right - now it's fixed.

Also, since I'm already butchering the poor creature here, what do you, @tsalo, and @physiopy/phys2denoise think about the functions and classes in peakdet.modalities and peakdet.analytics? I'd say that we could remove them from here and add what is missing or makes sense to add to phys2denoise. WDYT?

[tsalo]

At first glance, I think the code could be very useful in phys2denoise, but we would probably want to convert it to a functional approach to match phys2denoise's style.

[62442katieb]

Yes to peakdet.modalities, but the only thing in peakdet.analytics is heart rate variabilty computations. Are those used for denoising? (genuine question, as my knowledge of HR-based denoising has some gaps)

[smoia]

I genuinely don't know. In any case, I'll remove them - they're going to be in history.

[rmarkello]

(No, generally HRV metrics are used in analysis, not processing / denoising. I'd say it's safe to remove them if you're focusing the module on just processing 👍)

[smoia]

Ah, thank you @rmarkello!

[tsalo]

There probably should be a place within the physiopy umbrella for calculating metrics for analysis rather than denoising, right? Do we have a planned place for things like that?

[smoia]

If the analysis is MRI related, then phys2denoise would be the place for it (at least for the moment). If it's physiological signals only, probably other libraries would be more indicated (e.g. NeuroKit).

[CesarCaballeroGaudes]

Yes, HRV has only been proposed for analysis, not denoising.