Skip to content
New issue

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

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

Already on GitHub? # to your account

Let event selection dialogs show event labels #302

Merged
merged 11 commits into from
Mar 2, 2022

Conversation

hofaflo
Copy link
Contributor

@hofaflo hofaflo commented Feb 15, 2022

This lets dialogs in which users can select events show the event label instead of the integer id. If events aren't generated from annotations, but imported from a file, no event mapping exists, so the ids are shown. A future PR could introduce the possibility to change the event mapping.

@cbrnr
Copy link
Owner

cbrnr commented Feb 16, 2022

What about showing both event IDs and labels, e.g.

1 (T0)
2 (T1)
3 (T2)

If labels (mappings) are not present, only event IDs are shown.

Also, this change is currently only affecting the Epochs dialog. Were you planning to add more changes? If not, then the changelog entry should be modified to reflect what was actually changed.

@hofaflo
Copy link
Contributor Author

hofaflo commented Feb 25, 2022

What about showing both event IDs and labels, e.g.

👍

Also, this change is currently only affecting the Epochs dialog. Were you planning to add more changes? If not, then the changelog entry should be modified to reflect what was actually changed.

By passing the mapping to mne.Epochs, it's automatically reflected in the plotting dialogs as well.

@cbrnr
Copy link
Owner

cbrnr commented Feb 25, 2022

Great! Now the only thing I'd add is an option to let users modify the mapping. Maybe this could be done in the "Edit Events" dialog (including a "Clear mapping" button)? Otherwise I fear that people might be wondering where these mappings come from and how to change/edit/clear them.

@hofaflo
Copy link
Contributor Author

hofaflo commented Feb 26, 2022

done!
image

@cbrnr
Copy link
Owner

cbrnr commented Feb 27, 2022

Great! I'm sorry I keep making new requests, but this is a really long dialog. I'd put the mapping into its own sub-dialog, accessible through a button "Change mapping..." (to the left in the same row as OK/Cancel). If you don't want to implement it let me know and I'll do it.

@hofaflo
Copy link
Contributor Author

hofaflo commented Feb 28, 2022

[...] I'll do it.

Yes, please do!

@cbrnr cbrnr merged commit 2548b90 into cbrnr:main Mar 2, 2022
@cbrnr
Copy link
Owner

cbrnr commented Mar 2, 2022

Thanks @hofaflo!

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants