Check arguments for populate_intended_for functions only at top level

Each of the auxiliary functions don't need to do it again.

Author: pvelasco

heudiconv/bids.py

@@ -585,6 +585,8 @@ def find_fmap_groups(fmap_dir):
 def get_key_info_for_fmap_assignment(json_file, matching_parameter):
     """
     Gets key information needed to assign fmaps to other modalities.
+    (Note: It is the responsibility of the calling function to make sure
+    the arguments are OK)
 
     Parameters:
     ----------
@@ -603,10 +605,6 @@ def get_key_info_for_fmap_assignment(json_file, matching_parameter):
         raise FileNotFoundError(
             errno.ENOENT, os.strerror(errno.ENOENT), json_file
         )
-    if matching_parameter not in AllowedFmapParameterMatching:
-        raise ValueError(
-            "Fmap matching_parameter %s not allowed." % matching_parameter
-        )
 
     # loop through the possible criteria and extract the info needed
     if matching_parameter == 'Shims':
@@ -636,13 +634,18 @@ def get_key_info_for_fmap_assignment(json_file, matching_parameter):
         # We want to force the matching, so just return some string
         # regardless of the image
         key_info = [KeyInfoForForce]
+    else:
+        # fallback:
+        key_info = []
 
     return key_info
 
 
 def find_compatible_fmaps_for_run(json_file, fmap_groups, matching_parameters):
     """
     Finds compatible fmaps for a given run, for populate_intended_for.
+    (Note: It is the responsibility of the calling function to make sure
+    the arguments are OK)
 
     Parameters:
     ----------
@@ -662,16 +665,9 @@ def find_compatible_fmaps_for_run(json_file, fmap_groups, matching_parameters):
         key: prefix common to the group
         value: list of all fmap paths in the group
     """
-    if type(matching_parameters) is not list:
-        matching_parameters = [matching_parameters]
-
     lgr.debug('Looking for fmaps for %s', json_file)
     json_info = {}
     for param in matching_parameters:
-        if param not in AllowedFmapParameterMatching:
-            raise ValueError(
-                "Fmap matching_parameter %s not allowed." % param
-            )
         json_info[param] = get_key_info_for_fmap_assignment(json_file, param)
 
     compatible_fmap_groups = {}
@@ -698,6 +694,8 @@ def find_compatible_fmaps_for_run(json_file, fmap_groups, matching_parameters):
 def find_compatible_fmaps_for_session(path_to_bids_session, matching_parameters):
     """
     Finds compatible fmaps for all non-fmap runs in a session.
+    (Note: It is the responsibility of the calling function to make sure
+    the arguments are OK)
 
     Parameters:
     ----------
@@ -712,15 +710,6 @@ def find_compatible_fmaps_for_session(path_to_bids_session, matching_parameters)
     compatible_fmap : dict
         Dict of compatible_fmaps_groups (values) for each non-fmap run (keys)
     """
-    if not isinstance(matching_parameters, list):
-        assert isinstance(matching_parameters, str), "matching_parameters must be a str or a list, got %s" % matching_parameters
-        matching_parameters = [matching_parameters]
-    for param in matching_parameters:
-        if param not in AllowedFmapParameterMatching:
-            raise ValueError(
-                "Fmap matching_parameter %s not allowed." % param
-            )
-
     lgr.debug('Looking for fmaps for session: %s', path_to_bids_session)
 
     # Resolve path (eliminate '..')
@@ -753,6 +742,8 @@ def select_fmap_from_compatible_groups(json_file, compatible_fmap_groups, criter
     """
     Selects the fmap that will be used to correct for distortions in json_file
     from the compatible fmap_groups list, based on the given criterion
+    (Note: It is the responsibility of the calling function to make sure
+    the arguments are OK)
 
     Parameters:
     ----------
@@ -768,11 +759,6 @@ def select_fmap_from_compatible_groups(json_file, compatible_fmap_groups, criter
     selected_fmap_key : str or os.path
         key from the compatible_fmap_groups for the selected fmap group
     """
-    if criterion not in AllowedCriteriaForFmapAssignment:
-        raise ValueError(
-            "Fmap assignment criterion '%s' not allowed." % criterion
-        )
-
     if len(compatible_fmap_groups) == 0:
         return None
     # if compatible_fmap_groups has only one entry, that's it:
@@ -862,7 +848,8 @@ def populate_intended_for(path_to_bids_session, matching_parameters, criterion):
         fmaps to use
     """
 
-    if type(matching_parameters) is not list:
+    if not isinstance(matching_parameters, list):
+        assert isinstance(matching_parameters, str), "matching_parameters must be a str or a list, got %s" % matching_parameters
         matching_parameters = [matching_parameters]
     for param in matching_parameters:
         if param not in AllowedFmapParameterMatching:

heudiconv/tests/test_bids.py

@@ -144,10 +144,9 @@ def test_get_key_info_for_fmap_assignment(tmpdir):
         )
 
     # Finally: invalid matching_parameters:
-    with pytest.raises(ValueError):
-        assert get_key_info_for_fmap_assignment(
+    assert get_key_info_for_fmap_assignment(
             json_name, matching_parameter='Invalid'
-        )
+        ) == []
 
 
 def generate_scans_tsv(session_struct):
@@ -709,7 +708,7 @@ def test_find_compatible_fmaps_for_run(tmpdir, simulation_function, match_param)
             compatible_fmaps = find_compatible_fmaps_for_run(
                 json_file,
                 expected_fmap_groups,
-                matching_parameters=match_param
+                matching_parameters=[match_param]
             )
             assert compatible_fmaps == expected_compatible_fmaps[json_file]
 
@@ -757,7 +756,7 @@ def test_find_compatible_fmaps_for_session(
     session_folder = op.join(str(tmpdir), folder)
     _, _, _, expected_compatible_fmaps = simulation_function(session_folder)
 
-    compatible_fmaps = find_compatible_fmaps_for_session(session_folder, matching_parameters=match_param)
+    compatible_fmaps = find_compatible_fmaps_for_session(session_folder, matching_parameters=[match_param])
 
     assert compatible_fmaps == expected_compatible_fmaps