Bucanl PR

[cll008]

31

[arnodelorme]

Is there a way we could hand pick the commits. I think there is an option to allow edit when you submit a pull request.

[cll008]

@arnodelorme probably true. Let me actually see if I can get @Andesha here to clue us in on which commits should be brought to the main branch and which are just in house fixes.

[arnodelorme]

Sounds good. If he does not answer, we can review them together.

Arno

On Sep 27, 2021, at 4:38 PM, Clement Lee @.***> wrote:

@arnodelorme probably true. Let me actually see if I can get @Andesha here to clue us in on which commits should be brought to the main branch and which are just in house fixes.

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

[Andesha]

hello! I am available to discuss things, but I believe my colleague and I will need to re-familiarize with the code as it has been quite awhile. I will have them post here and then get back to you later this week.

[cll008]

That's great Tyler, thanks so much and keep us posted.

[arnodelorme]

Thanks Tyler. Looking forward to hear from you by the end of the week. This repo is very important to us.

Cheers, Arno

On Sep 28, 2021, at 10:51 AM, Clement Lee @.***> wrote:

That's great Tyler, thanks so much and keep us posted.

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

[SaraStephenson]

Hello, Tyler has brought this conversation to my attention and we have started to review the code and commits.

Best, Sara

[Andesha]

Here's a list of our current thoughts:

• JSONio
  • The JSON package most likely no longer matters
  • It has been quite some time, but we believe JSONio did not support writing of nested matlab structures to file at the time
  • As long as the JSON package you are using supports it, it should not matter
• pop_exportbids.m
  • We never used the internal EEGLAB study structure, so using your version of the script makes the most sense
  • We only ever used EEG as an array, i.e. EEG[0] is the first in the study EEG[1] is the second, to simulate a study
• eegplugin_bids.m
  • Since this is just menu customization and pathing manipulation you can just use your version of course
• bids_export.m
  • This is of course the file with the bulk of the changes
  • It is very difficult just from the diff/meld to see what would be needed
  • Is there an ongoing feature from my version you would like to bring over?

Hope this clears up at least some of the conflicts. Perhaps a meeting would be best to discuss the complications surrounding the bids_export.m script.

Lastly we will of course be happy to test your version of the plugin once you deem in ready with the Face13 (James Desjardins') training data set.

[cll008]

Thanks Tyler and Sara for the effort and the summary. Arno and I just went through bids_export and will test some things out. We were wondering about the byJson gInfo field you added... is this also for internal use for your json handling? It's not something that is in (or that you are suggesting for) the BIDS spec is it?

[Andesha]

It is indeed not part of the standard.

Judging by my use of it in the code I believe I used it as a function parameter for notifying the exporter of when to handle special array cases. I have a very strong memory of battling how matlab treated structs/cells/arrays in json :laughing:

Most likely the json exporter you are using more modern and up to date and will support the operations you desire.

[cll008]

Thanks for the clarification!

[arnodelorme]

Tyler,

Would you mind trying again using the plugin (our version) to export James Desjardin file? We have added the option to export EDF from your code.

Cheers,

Arno

On Oct 5, 2021, at 6:53 AM, Tyler K. Collins @.***> wrote:

It is indeed not part of the standard.

Judging by my use of it in the code I believe I used it as a function parameter for notifying the exporter of when to handle special array cases. I have a very strong memory of battling how matlab treated structs/cells/arrays in json 😆

Most likely the json exporter you are using more modern and up to date and will support the operations you desire.

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

[Andesha]

Sure thing.

However, I'll have @SaraStephenson run it and report back :)

[Andesha]

Hi Arno,

I'm continuing this discussion here but perhaps it would be better to open an issue...

I see that there is ongoing development still for this plugin. That's great, but it appears as though the examples may be out of synchronization with the bids_export_example.m script. What would be great is if you could tag a commit as a release or provide us a specific commit ID you would like us to test/clone against.

We were able to get the base structure working after some modifications, however we are not sure how certain we can be that things are stable until a release.

Additionally, I will be opening a secondary issue regarding the running of EEGLAB from within a BIDS code folder, i.e. bids_project_name/code/eeglab.

Thanks, Tyler