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

eda_plot throws ValueError when the number of onsets is not the same as the number of peaks #632

Closed
Rvb0rob0t opened this issue Mar 30, 2022 · 4 comments
Labels
EDA 💦 EDA related stuff wontfix This will not be worked on

Comments

@Rvb0rob0t
Copy link
Contributor

Describe the bug
eda_plot throws ValueError when the number of onsets is not the same as the number of peaks because of the sentence

end_onset = pd.Series(eda_signals["EDA_Phasic"][onsets].values, eda_signals["EDA_Phasic"][peaks].index)

To Reproduce

  1. Get a signal with a peak without an onset, like
[0.0,
 0.180624,
 0.217774,
 0.25108,
 0.228022,
 0.226741,
 0.226741,
 0.219055,
 0.217774,
 0.21265]
  1. Process it without cleaning like in Problem with EDA analysis because of low sampling rate (Empatica E4) #554 (I'm also using an Empatica E4)
  2. Try to plot the resulting signals with eda_plot()
  3. See error

Expected behaviour
I would expect one of the following:

  • that in this case, eda_plot would simply not plot the first onset.
  • that it warns that the specific fact that there is a peak without an onset is a problem.

System Specifications

  • OS: Linux (ELF 64bit)

  • Python: 3.10.2

  • NeuroKit2: 0.1.7

  • NumPy: 1.22.3

  • Pandas: 1.4.1

  • SciPy: 1.8.0

  • sklearn: 1.0.2

  • matplotlib: 3.5.1

@welcome
Copy link

welcome bot commented Mar 30, 2022

Hi 👋 Thanks for reaching out and opening your first issue here! We'll try to come back to you as soon as possible. ❤️ kenobi

@DominiqueMakowski
Copy link
Member

that in this case, eda_plot would simply not plot the first onset.

Yes we should go ahead with this option, I don't think there is a need for a warning.

Would you mind opening another PR with a fix? Thanks a lot!

@stale
Copy link

stale bot commented Sep 20, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

@stale stale bot added the wontfix This will not be worked on label Sep 20, 2022
@DominiqueMakowski DominiqueMakowski added EDA 💦 EDA related stuff and removed wontfix This will not be worked on labels Sep 21, 2022
@stale
Copy link

stale bot commented Nov 25, 2022

Is this still relevant? If so, what is blocking it? Is there anything you can do to help move it forward?

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
EDA 💦 EDA related stuff wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

2 participants