Skip to content

Reading events info from ceo file #9381

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

Merged
merged 11 commits into from
May 10, 2021
Merged

Reading events info from ceo file #9381

merged 11 commits into from
May 10, 2021

Conversation

dddd1007
Copy link
Contributor

@dddd1007 dddd1007 commented May 7, 2021

Reference issue

Fixes #9380

What does this implement/fix?

The read_raw_curry() cannot read the event file from .ceo file. So I try to make the function recognize .ceo files, a type of event file generated by Curry.

This function may cannot read event info from .cdt.ceo file.
Here is the issue about the fix. mne-tools#9380
@welcome
Copy link

welcome bot commented May 7, 2021

Hello! 👋 Thanks for opening your first pull request here! ❤️ We will try to get back to you soon. 🚴🏽‍♂️

@drammock
Copy link
Member

drammock commented May 7, 2021

@dddd1007 I've pushed a commit here that I think will fix your problem. Can you git pull this change and test it out?

@dddd1007
Copy link
Contributor Author

dddd1007 commented May 7, 2021

@drammock, I pulled this commit and tested it. read_raw_curry() can read all files correctly.

Thanks for your fixation!

Copy link
Member

@agramfort agramfort left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

diff looks good

I don't know how easy it would be to test this.

it will need a what's new entry too

however it's weird to open a PR first to maint branch. I think it should be merged to main and then backported

thx @dddd1007 and @drammock

@drammock
Copy link
Member

drammock commented May 8, 2021

it's weird to open a PR first to maint branch. I think it should be merged to main and then backported

I didn't even notice that, oops.

@larsoner larsoner changed the base branch from maint/0.23 to main May 10, 2021 12:11
@larsoner larsoner closed this May 10, 2021
@larsoner larsoner reopened this May 10, 2021
@larsoner
Copy link
Member

Changed the base branch and closed-reopened to restart PRs. But we have a bunch of PyVista-related errors so maybe we should fix those first, then do another restart here

@larsoner
Copy link
Member

@dddd1007 can you add an entry to changes/latest.inc while we wait for CIs to be fixed in #9386 and #9341?

@dddd1007
Copy link
Contributor Author

@dddd1007 can you add an entry to changes/latest.inc while we wait for CIs to be fixed in #9386 and #9341?

OK, I'll do it.

@larsoner larsoner mentioned this pull request May 10, 2021
Copy link
Member

@drammock drammock left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

merge when CIs green.

@larsoner larsoner merged commit 6afc722 into mne-tools:main May 10, 2021
@welcome
Copy link

welcome bot commented May 10, 2021

🎉 Congrats on merging your first pull request! 🥳 Looking forward to seeing more from you in the future! 💪

@larsoner
Copy link
Member

Thanks @dddd1007 !

@dddd1007 dddd1007 deleted the patch-1 branch May 11, 2021 00:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

mne.io.read_raw_curry() cannot read the event file end with '.cdt.ceo'
4 participants