[WIP] Replacing `paradim` with `events` or `experimental paradigm` as per BIDS #253

kchawla-pi · 2018-10-05T15:03:02Z

Must be merged only AFTER merging #238
For issue #216

- Added _make_localizer_first_level_paradigm_file_bids_compliant() to overwrite the downloaded events file with BIDS compliant version. - Added unit test for above function in nistats/tests/ - nistats/datasets.py imports reordered in compliance with PEP8.

…amples

- Renamed _make_localizer_first_level_paradigm_file_bids_compliant() to _make_bids_compliant_localizer_first_level_paradigm_file()

…liant.py - Renamed test_make_localizer_first_level_paradigm_file_bids_compliant.py to test_make_bids_compliant_localizer_first_level_paradigm_file.py

* master: utils._verify_events_file_uses_tab_separators() no longer returns any value Moved test file from nistats/tests/unit_tests/ to nistats/tests/ Corrected unit test filename to test_verify_events_file_uses_tab_separators Added unit tests for nistats._utils._verify_events_file_uses_tab_separators() Modified comments _verify_events_file_uses_tab_separators() returns a List[List[events, error]] Improved comments Renamed function parameter _not_test to _raise_delimiter_errors_only Renamed + Only first row is used to determine BIDS compliance of events files Added replacement function to raise error if events files use invalid separators Starting point for py3 to py2/3 conversion; Some reformatting Abandoned use of pathlib to maintain compatibility with older python versions Added function to raise error if events file doesn't use tabs or commas Removed unnecessary wrapper functions to read paradigm from tsv,csv files Removed unnecessary wrapper functions to read paradigm from tsv,csv files Added wrapper function to read experimental paradigm from tsv files

…parators() - Pre-mod BIDS compliance check is done by utils._verify_events_file_uses_tab_separators() TravisCI failure, possibly due to previously used function. - Removed datasets._check_bids_compliance_localizer_first_level_paradigm_file()

…ts file - Added datasets._make_spm_auditory_events_file() - Renamed _glob_spm_auditory_data() to _prepare_downloaded_spm_auditory_data() and refactored it out. - Refactored datasets.fetch_spm_auditory() into 3 functions: _download_spm_auditory_data(), _prepare_downloaded_spm_auditory_data(), _make_spm_auditory_events_file() .

…ats/tests/

…ment

- Added nistats.datasets._make_path_events_file_spm_auditory().

…st_datasets Changed the tutorial text in example plot_single_subject_run.py - Removed mention of events.csv . - Removed mention of events == paradigms.

- datasets._get_func_data_spm_multimodal() - datasets._get_session_trials_spm_multimodal() - datasets._get_anatomical_data_spm_multimodal() - datasets._glob_spm_multimodal_fmri_data() - datasets._download_data_spm_multimodal() - Added datasets._make_events_file_spm_multimodal_fmri() [INCOMPLETE]

…s in tests - Added _make_events_filepath_spm_multimodal_fmri(). - Events files are not egnerated during nosetests, and a warning is presented.

* master: doc fixes changed default oversampling to 50 exposed oversampling parameter

…faces.py - datasets.fetch_spm_multimodal_fmri() generates sessions from 1, not 0.

- Added nistats.utils._read_events_table()

…uch cleaner

…ppropriate - Minor documentation tweaks. - Replacing events access as key instead of attribute: data['events'] replaces data.events

bthirion

Thx, this is a great contribution, with only a few minor details.
However, several things are already included in open PRs. Will have to wait a bit before merging.

bthirion · 2018-10-05T20:49:07Z

nistats/experimental_paradigm.py

-    paradigm : pandas DataFrame
-        Describes a functional paradigm.
+    events : pandas DataFrame
+        Describes the events in a functional experimental paradigm.


Can remove "functional"

bthirion · 2018-10-05T20:49:19Z

examples/01_tutorials/plot_single_subject_single_run.py

-# We can read such a table from a spreadsheet file  created with OpenOffice Calcor Office Excel, and saved under the *comma separated values* format (``.csv``). 
+# timing of the auditory stimulation and rest periods (events).
+# This is typically provided in an events.tsv file. The path of this file is
+# provided in the dataset.
 import pandas as pd


Amigo, we're going to have a big clash with PR #231. This one will have to go after.

bthirion · 2018-10-05T20:49:51Z

nistats/design_matrix.py

-        A paradigm is considered as valid whenever it has an 'onset' key, if
-        this key is missing an exception will be thrown. For the others keys
-        only a simple warning will be displayed. A particular attention should
+        The events data is considered valid whenever it has an 'onset' key.


bthirion · 2018-10-05T20:50:05Z

nistats/design_matrix.py

-        Description of the experimental paradigm. The DataFrame instance might
-        have those keys:
+    events : DataFrame instance, optional
+        Events data that describes the experimental paradigm. The DataFrame instance might


bthirion · 2018-10-05T20:50:45Z

nistats/experimental_paradigm.py

-    if 'modulation' in paradigm.keys():
-        warnings.warn("'modulation' key not found in the given paradigm.")
-        modulation = np.array(paradigm['modulation']).astype(np.float)
+    if 'trial_type' in events.keys():


Oops, this is handled in PR #236 ...
Again, this one will have to go after #236 --which i am going to merge during the WE.

bthirion · 2018-10-05T20:53:53Z

nistats/utils.py

@@ -23,15 +23,39 @@ def _check_list_length_match(list_1, list_2, var_name_1, var_name_2):
            % (str(var_name_1), len(list_1), str(var_name_2), len(list_2)))


+def _read_events_table(table):


Isn't this featue used in many locations of the codebase ?

Didn't understand your suggestion.

I created this today because we now have two types of table-like files:

csv with index_col=0. (Many tests are also using such data)

tsv with index_col=False (BIDS compliant events files)

Are you saying I should unprotect it (remove the undescore) and make it a regular, reusable function?

unprotect the function -> Only if it indeed used somewhere else.
(I was just wondering)

bthirion · 2018-10-05T21:32:20Z

OK, I have started merging branches to master, which elicits these conflicts :-(

kchawla-pi · 2018-10-05T22:02:56Z

Yeah, no worries, we'll resolve this one once you are done with those merges.

kchawla-pi · 2018-10-05T22:07:15Z

There are lots of potential merge conflicts. I will resume work on this one on Sunday, until then most of the other PRs will likely be merged

kchawla-pi · 2018-10-08T12:26:41Z

The PR is transferred to #257

kchawla-pi added 21 commits October 1, 2018 16:51

Events file for localizer_first_level is no longer modified in/for ex…

0bf3715

…amples

localizer_first_level events file not modified if already BIDS compliant

b5a0293

Renamed _make_localizer_first_level_paradigm_file_bids_compliant()

69c78ce

- Renamed _make_localizer_first_level_paradigm_file_bids_compliant() to _make_bids_compliant_localizer_first_level_paradigm_file()

Renamed tests/test_make_localizer_first_level_paradigm_file_bids_comp…

749af04

…liant.py - Renamed test_make_localizer_first_level_paradigm_file_bids_compliant.py to test_make_bids_compliant_localizer_first_level_paradigm_file.py

Added unit test for datasets._make_spm_auditory_events_file() in nist…

df13582

…ats/tests/

Modified example to work with BIDS events file for spm_auditory dataset

b93152a

Removed test of filename check, since it doesn't work in a CI environ…

7fb6d7f

…ment

Refactored events filepath creation

2c6c249

- Added nistats.datasets._make_path_events_file_spm_auditory().

Moved test_make_bids_compliant_locizer & test_make_spm_auditory to te…

68c223f

…st_datasets Changed the tutorial text in example plot_single_subject_run.py - Removed mention of events.csv . - Removed mention of events == paradigms.

Tweaked function names & docstrings

3744764

datasets.fetch_spm_multimodal_fmri() generates events.tsv files; warn…

873bb76

…s in tests - Added _make_events_filepath_spm_multimodal_fmri(). - Events files are not egnerated during nosetests, and a warning is presented.

Merge branch 'master' into bids-compliant-fetchers

77701c9

* master: doc fixes changed default oversampling to 50 exposed oversampling parameter

Removed events data generating code from example plot_spm_multimodal_…

6addef0

…faces.py - datasets.fetch_spm_multimodal_fmri() generates sessions from 1, not 0.

tsv and csv files are now correctly loaded as pandas dataframe

50a4908

- Added nistats.utils._read_events_table()

Example uses pd.read_table() for .tsv, as has correct default args; m…

b1b859a

…uch cleaner

Term paradigm replaced by experimental paradigms or events as a…

8924964

…ppropriate - Minor documentation tweaks. - Replacing events access as key instead of attribute: data['events'] replaces data.events

bthirion reviewed Oct 5, 2018

View reviewed changes

kchawla-pi closed this Oct 8, 2018

kchawla-pi deleted the events-not-paradigms-2 branch October 8, 2018 17:21

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[WIP] Replacing `paradim` with `events` or `experimental paradigm` as per BIDS #253

[WIP] Replacing `paradim` with `events` or `experimental paradigm` as per BIDS #253

kchawla-pi commented Oct 5, 2018 •

edited

bthirion left a comment

bthirion Oct 5, 2018

bthirion Oct 5, 2018

bthirion Oct 5, 2018

bthirion Oct 5, 2018

bthirion Oct 5, 2018

bthirion Oct 5, 2018

kchawla-pi Oct 5, 2018

bthirion Oct 7, 2018

bthirion commented Oct 5, 2018

kchawla-pi commented Oct 5, 2018

kchawla-pi commented Oct 5, 2018

kchawla-pi commented Oct 8, 2018

		@@ -23,15 +23,39 @@ def _check_list_length_match(list_1, list_2, var_name_1, var_name_2):
		% (str(var_name_1), len(list_1), str(var_name_2), len(list_2)))


		def _read_events_table(table):

[WIP] Replacing paradim with events or experimental paradigm as per BIDS #253

[WIP] Replacing paradim with events or experimental paradigm as per BIDS #253

Conversation

kchawla-pi commented Oct 5, 2018 • edited

bthirion left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bthirion commented Oct 5, 2018

kchawla-pi commented Oct 5, 2018

kchawla-pi commented Oct 5, 2018

kchawla-pi commented Oct 8, 2018

[WIP] Replacing `paradim` with `events` or `experimental paradigm` as per BIDS #253

[WIP] Replacing `paradim` with `events` or `experimental paradigm` as per BIDS #253

kchawla-pi commented Oct 5, 2018 •

edited