Data format MEF 2.1, 3.0 and MED 1.0 importer #2379
Conversation
Refactor code for better performance Add new feature to search functionality Update UI design for better user experience Fix typo in variable name Add error handling for edge case scenario Optimize database queries for faster response time Update documentation for new API endpoints Remove unused code Fix formatting issues in code
|
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_bug1248, test_issue2021, test_issue627, test_pull1600, test_tutorial_preprocessing, test_bug2370, test_bug1262, test_tutorial_meguk2015, test_pull2050, test_issue1311, test_yorkinstruments, test_bug1142, test_bug1397, test_tutorial_eventrelatedaveraging, test_bug2231, test_bug2482, test_bug1404, test_bug97, test_issue1585, test_tutorial_timefrequencyanalysis20130308, test_pull731, test_read_trigger, test_bug1408, test_bug2590, test_tutorial_timefrequencyanalysis, test_bug2956, test_bug3075, test_pull1331, test_example_incorporate_head_movements, test_bug804, test_tutorial_eeg_preprocessing, test_firwsfiltering, inspect_issue1216, test_ft_combineplanar, test_bug2277, test_bug1571, test_bug2269 When inside the DCCN, please also consider testing: test_bug2887, test_bug629, test_bug2170, test_bug182, test_ft_read_header, test_bug1665, test_bug1359, test_bug2027, test_bug2385, test_issue789, test_bug1403, test_bug1807, test_issue1425, test_bug1856, test_bug1262, test_readcompresseddata, test_tutorial_meguk2015, test_bug2303, test_issue1601, test_bug1412, test_issue2026, test_yorkinstruments, test_issue1320, test_ctf2grad, test_bug2060, inspect_bug645, test_bug1407, test_bug1924, test_bug2231, test_issue1991, test_issue1387, test_bug1306, test_bug1266, test_bug2482, test_ft_preprocessing, test_bug97, test_issue1585, test_pull731, test_bug1129, test_bug1808, test_bug1490b, test_bug1490, test_bug2888, test_bug1914, test_bug3207, test_bug2220, test_bug2093, test_bug2415, test_bug1348, test_bug963, test_pull2111, test_curry, test_pull832, test_bug1667, test_pull1271, test_pull769, test_bug1125, test_bug804, test_pull1248, inspect_issue1216, test_bug3027, test_issue1972, test_bug1998, test_issue1992, test_pull1229, test_bug1427, test_bug1571, test_pull574_reref Suggested tests outside the DCCN use public data or do not use data. |
There was a problem hiding this comment.
ft_hastoolbox has different changes at different locations (i.e. in utlities/ft_hastoolbox, and in fileio/private/ft_hastoolbox). Obviously, these changes should be the same. Note that there are mechanisms in place that synchronize different copies of the same function across the codebase, so the change needs to be made only on one of the files.
|
Can you please let me know the "mechanism" (perhaps a link to the corresponding website) that can synchronize the different copies of the same function? Thanks! |
|
There's a shell script: bin/synchronize_private.sh that takes care that changes in one version of a function end up everywhere. This will be deployed automatically once a change has been merged. You don't need to do anything, just ensure that only one copy of the files contains the 'union' of all changes that now seem to be distributed among the two different versions. |
|
OK. Thanks! I'll double-check function ft_hastoolbox to make sure only one copy has the requested changes. |
|
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_bug1142, test_bug1262, test_bug3075, test_bug2956, test_bug2277, test_issue1585, test_issue1311, test_tutorial_preprocessing, test_bug2370, test_bug2231, inspect_issue1216, test_pull2050, test_bug2590, test_pull1600, test_tutorial_timefrequencyanalysis, test_issue627, test_pull1331, test_tutorial_timefrequencyanalysis20130308, test_issue2021, test_yorkinstruments, test_tutorial_meguk2015, test_bug1248, test_bug2269, test_bug1571, test_firwsfiltering, test_bug1404, test_tutorial_eeg_preprocessing, test_bug2482, test_pull731, test_ft_combineplanar, test_example_incorporate_head_movements, test_bug1408, test_bug1397, test_read_trigger, test_tutorial_eventrelatedaveraging, test_bug804, test_bug97 When inside the DCCN, please also consider testing: test_issue789, test_bug1490b, test_bug2093, test_pull574_reref, test_issue2026, test_bug1412, test_pull769, test_bug1856, test_bug1348, test_bug1125, test_bug1262, test_bug1306, test_bug1359, test_bug629, inspect_bug645, test_issue1991, test_ft_read_header, test_bug2231, test_bug1129, inspect_issue1216, test_curry, test_pull1271, test_bug2887, test_pull832, test_bug1490, test_pull1229, test_bug1667, test_ctf2grad, test_bug3207, test_bug1407, test_pull2111, test_bug2385, test_bug1914, test_issue1992, test_issue1601, test_bug1807, test_issue1320, test_bug2415, test_yorkinstruments, test_tutorial_meguk2015, test_bug1571, test_bug2303, test_bug1403, test_pull1248, test_issue1387, test_bug2482, test_bug2060, test_bug1998, test_readcompresseddata, test_bug2220, test_pull731, test_bug1266, test_bug2027, test_bug182, test_bug1427, test_bug1924, test_issue1972, test_bug1808, test_bug2170, test_bug2888, test_bug3027, test_issue1425, test_bug804, test_issue1585, test_bug97, test_bug1665, test_bug963, test_ft_preprocessing Suggested tests outside the DCCN use public data or do not use data. |
|
You should test whether your modifications do not break anything. When outside the DCCN, please consider testing: test_bug2269, test_pull2050, test_tutorial_eeg_preprocessing, test_bug97, test_read_trigger, test_bug2370, test_bug1571, test_bug2956, test_pull731, test_tutorial_timefrequencyanalysis20130308, test_firwsfiltering, test_issue1585, test_bug1408, test_pull1331, test_issue2021, test_tutorial_preprocessing, test_issue1311, test_example_incorporate_head_movements, test_bug2482, test_pull1600, test_bug2277, test_bug1142, inspect_issue1216, test_bug1248, test_bug3075, test_bug804, test_bug2231, test_tutorial_meguk2015, test_bug1397, test_ft_combineplanar, test_bug1404, test_yorkinstruments, test_bug1262, test_issue627, test_tutorial_eventrelatedaveraging, test_tutorial_timefrequencyanalysis, test_bug2590 When inside the DCCN, please also consider testing: test_ft_read_header, test_bug1665, test_issue1601, test_bug1914, test_bug97, test_issue1972, test_issue1991, test_bug1807, test_pull2111, test_bug1571, test_bug629, test_pull731, test_bug1412, test_bug1998, test_bug2220, test_issue1585, test_pull832, test_bug182, test_bug2385, test_bug2887, test_curry, test_bug1403, test_pull574_reref, test_bug963, test_bug1856, test_bug1407, test_bug3027, test_ft_preprocessing, test_bug2303, test_bug2482, test_issue1425, test_bug1348, test_bug2027, test_bug1490, test_bug2170, test_readcompresseddata, test_pull1248, inspect_issue1216, test_issue1992, test_ctf2grad, test_bug1306, inspect_bug645, test_bug1924, test_bug804, test_bug2415, test_bug1808, test_issue789, test_issue1320, test_bug2231, test_issue2026, test_bug1129, test_tutorial_meguk2015, test_bug3207, test_bug1427, test_pull1229, test_bug2093, test_pull769, test_yorkinstruments, test_bug1266, test_bug2888, test_bug1667, test_bug1125, test_bug1262, test_issue1387, test_bug2060, test_pull1271, test_bug1490b, test_bug1359 Suggested tests outside the DCCN use public data or do not use data. |
|
Hi, I have checked the code to make sure "utlities/ft_hastoolbox" and "fileio/private/ft_hastoolbox" are identical. Please let me know whether you can merge the changes now. Thanks! |
|
OK, thanks. Yet, from a conceptual point of view, I am wondering how we should proceed here. Perhaps @robertoostenveld can chime in. As I see it, this PR actually wants to merge code into FieldTrip that is currently in another repo: https://github.com/jiecui/mef_reader_fieldtrip/tree/master. The given repo, besides containing an external folder, which is the beef of the current PR here, also has some overloaded copies of fileio functions from fieldtrip ft_read_data/event/header, and ft_preprocessing, which I think also are included in this PR. Now, the question is who is going to take responsibility for keeping the repos in sync, and minimizing the risk of users ending up with ill-specified or incompatible sub-parts of the code on their paths. I see a risk in you (or somebody else) upgrading/improving sub-functionality of the mef_read_fieldtrip code, but these changes might not percolate automatically to fieldtrip/external/mayo_mef. The installation instructions are detailed, but also a bit confusing, because they aim at accommodating various potential FieldTrip version specific issues, and the installation instruction includes some 'band aids' that suggest to copy over FieldTrip files from your repo into fieldtrip root, and also to move around the mayo_mef folders. I think that this is unnecessarily complicated (and error prone). To me, it looks that the external folder is well designed, yet best supported at its origin (thus, not as an external folder in the FieldTrip repo), and that the PR here should focus on providing the glue code in the relevant fieldtrip functions (i.e. ft_read_data/event/header, and ft_hastoolbox) + documentation w.r.t. what is needed to get started with this type of data. This would then obviously require instructions to include an installation of (an adjusted version of your) mef_reader_fieldtrip repo + the other stuff that you have already documented in your installation instructions. Given that a user already needs to download the MED reader from an external source, I don't think that this will lead to much additional overhead (and it would greatly simplify steps 2/4/5 in your installation instructions). So, all in all, my suggestion for moving forward would be to: This would I think give the best balance in code maintainability in the future, and also gives sufficient credit to @jiecui I am curious about what the two of you think about this. |
This is an update for importing neural data format MEF 2.1, 3.0 and MED 1.0 into FieldTrip.