MaxvandenBoom / matmef

Matlab wrapper for MEF library
GNU General Public License v3.0
1 stars 3 forks source link

EEGLAB plugin implementation #3

Closed arnodelorme closed 2 years ago

arnodelorme commented 2 years ago

The additional code allows to import files in EEGLAB.

MaxvandenBoom commented 2 years ago

Great, thanks for the addition! I'll check it this week and merge.

Do you think it is possible to remove the binary from this pull request? There's a couple of other things I wil update/include this week and I'll update the binaries (mac and windows both) in one-go.

Also I'll set a github version/release for matmef after the coming week's update, guess it has matured enough to count as initial (1.0) From your commit, do I understand correctly that you want to keep your own version count for the EEGLab plugin within matmef?

arnodelorme commented 2 years ago

Great, thanks for the addition! I'll check it this week and merge.

Do you think it is possible to remove the binary from this pull request?

Oh, yes, sorry about that. But because they are small, they should be included. Most people will not know how to recompile these files. I have left them for now.

There's a couple of other things I wil update/include this week and I'll update the binaries (mac and windows both) in one-go.

Also I'll set a github version/release for matmef after the coming week's update, guess it has matured enough to count as initial (1.0) From your commit, do I understand correctly that you want to keep your own version count for the EEGLab plugin within matmef?

Perfect.

Cheers,

Arno

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

MaxvandenBoom commented 2 years ago

But because they are small, they should be included. Most people will not know how to recompile these files. I have left them for now.

I understand and agree! I have already always included the binary files in this repo for both windows and mac. You can see, the mex functions read_mef_ts_data and read_mef_session_metadata are already there as .mexmaci64and .mexw64. So feel free to remove your binary from the pull-request.

Best, Max

MaxvandenBoom commented 2 years ago

Hi Arno,

I released the current repo state as v1.0.0 and will use github releases (from now on) as a means of versioning.

I prefer to not have the version changes in the README file. Instead, I will add them to the description in github upon each new release (including updates to the EEGLIB plugin). Meaning that once we have merged this EEGLab addition and are satisfied with it's workings, I will add the "initial release for EEGLAB plugin" line to the description of the next release.

Could you please remove the "version" adjustments you made to the readme file. Offcourse what you added to the acknowledgments should stay in and will be merged as it is :)

arnodelorme commented 2 years ago

Hi Max,

I tried removing the binary but it is not that simple. I would need to copy the old binary but even so, the Git history will choke and resubmit it to you these old files as new files (even though the binary are identical). In any case, simply copy and resubmit the two files to your repo, ignoring the pull request.

https://github.com/arnodelorme/matmef/blob/master/eegplugin_matmef.m

https://github.com/arnodelorme/matmef/blob/master/pop_matmef.m

If you issue a new Github release, make sure the version number in (https://github.com/arnodelorme/matmef/blob/master/eegplugin_matmef.m) is the same as the GitHub release for consistency. If you could submit the plugin to

https://sccn.ucsd.edu/eeglab/plugin_uploader/upload_form.php

That would be fantastic.

Cheers,

Arno

On Sep 28, 2021, at 8:37 AM, Max @.***> wrote:

Hi Arno,

I released the current repo state as v1.0.0 and will use github releases (from now on) as a means of versioning.

I prefer to not have the version changes in the README file. Instead, I will add them to the description in github upon each new release (including updates to the EEGLIB plugin). Meaning that once we have merged this EEGLab addition and are satisfied with it's workings, I will add the "initial release for EEGLAB plugin" line to the description of the next release.

Could you please remove the "version" adjustments you made to the readme file. Offcourse what you added to the acknowledgments should stay in and will be merged as it is :)

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

MaxvandenBoom commented 2 years ago

Hi Max, I tried removing the binary but it is not that simple. I would need to copy the old binary but even so, the Git history will choke and resubmit it to you these old files as new files (even though the binary are identical). In any case, simply copy and resubmit the two files to your repo, ignoring the pull request.

Aah, I would have liked to have you as a contributor in this repo that comes with the pull request :) I can do it the way you propose. Alternatively you can add me as a collaborator to your fork and I will fix the conflicts for you.

If you issue a new Github release, make sure the version number in (https://github.com/arnodelorme/matmef/blob/master/eegplugin_matmef.m) is the same as the GitHub release for consistency.

I expected that, I will.

If you could submit the plugin to https://sccn.ucsd.edu/eeglab/plugin_uploader/upload_form.php That would be fantastic.

Will do when it's done!

arnodelorme commented 2 years ago

On Sep 28, 2021, at 9:42 AM, Max @.***> wrote:

Hi Max, I tried removing the binary but it is not that simple. I would need to copy the old binary but even so, the Git history will choke and resubmit it to you these old files as new files (even though the binary are identical). In any case, simply copy and resubmit the two files to your repo, ignoring the pull request.

Aah, I would have liked to have you as a contributor in this repo that comes with the pull request :)

Thanks. It is not important though so do not worry about it.

I can do it the way you propose. Alternatively you can add me as a collaborator to your fork and I will fix the conflicts for you.

I have added you to the repo.

If you issue a new Github release, make sure the version number in (https://github.com/arnodelorme/matmef/blob/master/eegplugin_matmef.m) is the same as the GitHub release for consistency.

I expected that, I will.

If you could submit the plugin to https://sccn.ucsd.edu/eeglab/plugin_uploader/upload_form.php That would be fantastic.

Will do when it's done!

Wonderful. Cheers,

Arno

MaxvandenBoom commented 2 years ago

Alright, resolved and merged! Thanks for your contribution :)

Before I submit it as a plugin, have you tested the EEGLab code already on a MEF3 dataset? I'm asking since you were encountering some problems according to issue #2.

arnodelorme commented 2 years ago

It worked on Windows but not on Mac (I could not recompile on Mac and when I try using the stocked compiled file, I get a “beep” and Matlab hangs).

On Sep 28, 2021, at 10:50 AM, Max @.***> wrote:

Alright, resolved and merged! Thanks for your contribution :)

Before I submit it as a plugin, have you tested the EEGLab code already on a MEF3 dataset? I'm asking since you were encountering some problems according to issue #2.

— You are receiving this because you authored the thread. Reply to this email directly, view it on GitHub, or unsubscribe. Triage notifications on the go with GitHub Mobile for iOS or Android.

MaxvandenBoom commented 2 years ago

That's weird, I have never encountered the mac binary not working on any of the computer here in the lab :/

Could you try retrieving a fresh clone of Matmef and then using a low-level function to read the metadata like so: session = read_mef_session_metadata('/BIDS_MotorAmp/sub-01/ses-ieeg01/ieeg/sub-01_ses-ieeg01_task-finger_run-1234_ieeg.mefd', [], 1); Please make sure to also retrieve a fresh download of the dataset again from OSF (https://osf.io/vmxdn/), I updated it this morning to be complete.

I really appreciate your help in finding out what is happening here!

arnodelorme commented 2 years ago

Yes. That worked. Tx

MaxvandenBoom commented 2 years ago

@arnodelorme I submitted Matmef v1.1.0 as an EEGLab plugin just now, tnx!