Repository: bids-standard/pybids

Issue:
Allow wildcard names for model variables
I'm trying to pass variable names to a bids model with wildcards like so:

"Model": {
                    "X": [
                    'stimulus_type.neutral_anticipation',
                    'stimulus_type.neutral_feedback',
                    'stimulus_type.loss_anticipation',
                    'stimulus_type.avoided_loss_feedback',
                    'stimulus_type.gain_anticipation',
                    'stimulus_type.gain_feedback',
                    'stimulus_type.missed_gain_feedback',
                    'stimulus_type.loss_feedback',
                    'trans_*',
                    'rot_*',
                    'cosine*',
                    'a_comp_cor_*',
                    't_comp_cor_*',
                    'aroma_motion_*',
                    'censor_*'
                ],
            },

This is throwing a key error trans_*

```
Traceback (most recent call last):
  File "/data/nielsond/python/envs/fitlins_dev/lib/python3.6/site-packages/nipype/pipeline/plugins/multiproc.py", line 69, in run_node
    result['result'] = node.run(updatehash=updatehash)
  File "/data/nielsond/python/envs/fitlins_dev/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 473, in run
    result = self._run_interface(execute=True)
  File "/data/nielsond/python/envs/fitlins_dev/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 564, in _run_interface
    return self._run_command(execute)
  File "/data/nielsond/python/envs/fitlins_dev/lib/python3.6/site-packages/nipype/pipeline/engine/nodes.py", line 649, in _run_command
    result = self._interface.run(cwd=outdir)
  File "/data/nielsond/python/envs/fitlins_dev/lib/python3.6/site-packages/nipype/interfaces/base/core.py", line 376, in run
    runtime = self._run_interface(runtime)
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/fitlins/fitlins/interfaces/bids.py", line 235, in _run_interface
    analysis.setup(drop_na=False, desc='preproc', **selectors)
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/analysis.py", line 89, in setup
    b.setup(input_nodes, drop_na=drop_na, **selectors)
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/analysis.py", line 213, in setup
    transform.Select(coll, X)
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/transformations/base.py", line 87, in __new__
    return t.transform()
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/transformations/base.py", line 223, in transform
    self._clone_variables()
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/transformations/base.py", line 123, in _clone_variables
    for v in self.variables}
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/analysis/transformations/base.py", line 123, in <dictcomp>
    for v in self.variables}
  File "/gpfs/gsfs11/users/MBDU/midla/notebooks/code/pybids/bids/variables/kollekshuns.py", line 184, in __getitem__
    return self.variables[var]
KeyError: 'trans_*'
```

I get the same thing if i use trans_.*
I'm pretty sure this is happening because no matching is being done on X here:
https://github.com/bids-standard/pybids/blob/15aa08fb2b146e3fcc62f359ea4a4d810d38591a/bids/analysis/analysis.py#L212-L213

Discussion:

Patch:
diff --git a/bids/analysis/transformations/__init__.py b/bids/analysis/transformations/__init__.py
index 2abaf33f..861332e8 100644
--- a/bids/analysis/transformations/__init__.py
+++ b/bids/analysis/transformations/__init__.py
@@ -1,7 +1,7 @@
 from .compute import (Sum, Product, Scale, Orthogonalize, Threshold, And, Or,
                       Not, Demean, Convolve)
 from .munge import (Split, Rename, Assign, Copy, Factor, Filter, Select,
-                    Delete, DropNA, Replace, ToDense)
+                    Delete, DropNA, Replace, ToDense, Group)
 
 __all__ = [
     'And',
@@ -13,6 +13,7 @@ __all__ = [
     'DropNA',
     'Factor',
     'Filter',
+    'Group',
     'Not',
     'Or',
     'Orthogonalize',
diff --git a/bids/analysis/transformations/base.py b/bids/analysis/transformations/base.py
index b9e99fdf..03257db7 100644
--- a/bids/analysis/transformations/base.py
+++ b/bids/analysis/transformations/base.py
@@ -1,15 +1,19 @@
 """Base Transformation class and associated utilities. """
 
-import numpy as np
-import pandas as pd
-from bids.utils import listify
+import re
 import warnings
 from abc import ABCMeta, abstractmethod
 from copy import deepcopy
 import itertools
 import inspect
+
+import numpy as np
+import pandas as pd
+
+from bids.utils import listify
 from bids.variables import SparseRunVariable
 
+
 class Transformation(metaclass=ABCMeta):
 
     ### Class-level settings ###
@@ -106,10 +110,43 @@ class Transformation(metaclass=ABCMeta):
 
         self.kwargs = kwargs
 
-        # Expand regex variable names
-        replace_args = self.kwargs.pop('regex_variables', None)
-        if replace_args is not None:
-            self._regex_replace_variables(replace_args)
+        # Expand any detected variable group names or wild cards
+        self._expand_variable_groups()
+        self._expand_variable_names()
+
+    def _expand_variable_groups(self):
+        """ Replace any detected variable groups with the associated lists of
+        variable names.
+        """
+        groups = self.collection.groups
+        variables = [groups[v] if v in groups else [v] for v in self.variables]
+        self.variables = list(itertools.chain(*variables))
+
+    def _expand_variable_names(self):
+        """Filter all available arguments against collection's variables using
+        unix-style pattern matching."""
+        def _replace_arg_values(values):
+            is_iter = isinstance(values, (list, tuple))
+            values = listify(values)
+            result = []
+            # Only try to match strings containing a relevant special character
+            for v in values:
+                if isinstance(v, str) and re.search('[\*\?\[\]]', v):
+                    result.append(self.collection.match_variables(v))
+                else:
+                    result.append([v])
+
+            result = list(itertools.chain(*result))
+            # Don't return a list unless we have to
+            if is_iter or len(result) > 1:
+                return result
+            return result[0]
+
+        # 'variables' is stored separately, so handle it separately
+        self.variables = _replace_arg_values(self.variables)
+
+        for k, arg in self.kwargs.items():
+            self.kwargs[k] = _replace_arg_values(arg)
 
     def _clone_variables(self):
         """Deep copy all variables the transformation touches. This prevents us
@@ -170,39 +207,6 @@ class Transformation(metaclass=ABCMeta):
                 sr = self.collection.sampling_rate
                 self._variables[v] = var.to_dense(sr)
 
-    def _regex_replace_variables(self, args):
-        """For each argument named in args, interpret the values set in the
-        argument as regex patterns to potentially be replaced with variables
-        that match the pattern. """
-
-        args = listify(args)
-
-        if 'variables' in args:
-            args.remove('variables')
-            variables = True
-        else:
-            variables = False
-
-        # Ensure all keyword arguments user wants to scan are valid
-        missing = set(args) - set(self.kwargs.keys())
-        if missing:
-            raise ValueError("Arguments '%s' specified for regex-based "
-                             "variable name replacement, but were not found "
-                             "among keyword arguments." % missing)
-
-        def _replace_arg_values(names):
-            variables = listify(names)
-            variables = [self.collection.match_variables(c) for c in names]
-            variables = itertools.chain(*variables)
-            return list(set(variables))
-
-        # 'variables' is stored separately, so handle it separately
-        if variables:
-            self.variables = _replace_arg_values(self.variables)
-
-        for arg in args:
-            self.kwargs[arg] = _replace_arg_values(self.kwargs[arg])
-
     def transform(self):
 
         output_passed = not (self.output is None and self.output_prefix is None
diff --git a/bids/analysis/transformations/compute.py b/bids/analysis/transformations/compute.py
index 8e144e05..80f6232f 100644
--- a/bids/analysis/transformations/compute.py
+++ b/bids/analysis/transformations/compute.py
@@ -168,7 +168,7 @@ class Sum(Transformation):
             if len(weights.ravel()) != data.shape[1]:
                 raise ValueError("If weights are passed to sum(), the number "
                                  "of elements must equal number of variables"
-                                 "being summed.")
+                                 " being summed.")
         return (data * weights).sum(axis=1)
 
 
diff --git a/bids/analysis/transformations/munge.py b/bids/analysis/transformations/munge.py
index d969f33a..2e5c6a67 100644
--- a/bids/analysis/transformations/munge.py
+++ b/bids/analysis/transformations/munge.py
@@ -206,6 +206,21 @@ class Filter(Transformation):
         return var
 
 
+class Group(Transformation):
+    """Groups a list of variables."""
+
+    _groupable = False
+    _loopable = False
+    _input_type = 'variable'
+    _return_type = 'none'
+
+    def _transform(self, variables, name):
+        if name in self.variables:
+            raise ValueError("Variable group name '{}' conflicts with an "
+                             "existing variable name!".format(name))
+        self.collection.groups[name] = [v.name for v in variables]
+
+
 class Rename(Transformation):
     """Rename a variable.
 
@@ -268,8 +283,8 @@ class Select(Transformation):
 
 
 class Split(Transformation):
-    """Split a single variable into N variables as defined by the levels of one or
-    more other variables.
+    """Split a single variable into N variables as defined by the levels of one
+    or more other variables.
 
     Parameters
     ----------
@@ -284,7 +299,7 @@ class Split(Transformation):
     _allow_categorical = ('by',)
     _densify = ('variables', 'by')
 
-    def _transform(self, var, by, drop_orig=True):
+    def _transform(self, var, by):
 
         if not isinstance(var, SimpleVariable):
             self._densify_variables()
@@ -299,29 +314,13 @@ class Split(Transformation):
         group_data = pd.concat(by_variables, axis=1, sort=True)
         group_data.columns = listify(by)
 
-        # For sparse data, we need to set up a 1D grouper
-        if isinstance(var, SimpleVariable):
-            # Create single grouping variable by combining all 'by' variables
-            if group_data.shape[1] == 1:
-                group_labels = group_data.iloc[:, 0].values
-            else:
-                group_rows = group_data.astype(str).values.tolist()
-                group_labels = ['_'.join(r) for r in group_rows]
-
-            result = var.split(group_labels)
-
-        # For dense data, use patsy to create design matrix, then multiply
-        # it by target variable
-        else:
-            group_data = group_data.astype(str)
-            formula = '0+' + '*'.join(listify(by))
-            dm = dmatrix(formula, data=group_data, return_type='dataframe')
-            result = var.split(dm)
-
-        if drop_orig:
-            self.collection.variables.pop(var.name)
+        # Use patsy to create splitting design matrix
+        group_data = group_data.astype(str)
+        formula = '0+' + ':'.join(listify(by))
+        dm = dmatrix(formula, data=group_data, return_type='dataframe')
+        dm.columns = [col.replace(':', '.') for col in dm.columns]
 
-        return result
+        return var.split(dm)
 
 
 class ToDense(Transformation):
diff --git a/bids/variables/kollekshuns.py b/bids/variables/kollekshuns.py
index 6806f996..92f14e71 100644
--- a/bids/variables/kollekshuns.py
+++ b/bids/variables/kollekshuns.py
@@ -5,17 +5,20 @@ module of the same name on Python 2. We could go with something sensible but
 verbose like 'variable_collections', but that would make far too much sense.
 """
 
-import pandas as pd
 from copy import copy
-from pandas.api.types import is_numeric_dtype
 import warnings
 import re
-from .variables import (SparseRunVariable, SimpleVariable, DenseRunVariable,
-                        merge_variables, BIDSVariable)
 from collections import OrderedDict
 from itertools import chain
-from bids.utils import listify, matches_entities
+import fnmatch
+
 import numpy as np
+import pandas as pd
+from pandas.api.types import is_numeric_dtype
+
+from .variables import (SparseRunVariable, SimpleVariable, DenseRunVariable,
+                        merge_variables, BIDSVariable)
+from bids.utils import listify, matches_entities
 
 
 class BIDSVariableCollection(object):
@@ -67,6 +70,10 @@ class BIDSVariableCollection(object):
         self.variables = {v.name: v for v in variables}
         self._index_entities()
 
+        # Container for variable groups (see BIDS-StatsModels spec)--maps from
+        # group names to lists of variables.
+        self.groups = {}
+
     @staticmethod
     def merge_variables(variables, **kwargs):
         """Concatenates Variables along row axis.
@@ -214,8 +221,8 @@ class BIDSVariableCollection(object):
             obj.name = var
         self.variables[var] = obj
 
-    def match_variables(self, pattern, return_type='name'):
-        """Return columns whose names match the provided regex pattern.
+    def match_variables(self, pattern, return_type='name', match_type='unix'):
+        """Return columns whose names match the provided pattern.
 
         Parameters
         ----------
@@ -226,11 +233,19 @@ class BIDSVariableCollection(object):
             'name': Returns a list of names of matching variables.
             'variable': Returns a list of Variable objects whose names
             match.
+        match_type : str
+            Matching approach to use. Either 'regex' (full-blown regular
+                expression matching) or 'unix' (unix-style pattern matching
+                via the fnmatch module).
         """
-        pattern = re.compile(pattern)
-        vars_ = [v for v in self.variables.values() if pattern.search(v.name)]
-        return vars_ if return_type.startswith('var') \
-            else [v.name for v in vars_]
+        if match_type.lower().startswith('re'):
+            pattern = re.compile(pattern)
+            vars_ = [v for v in self.variables.keys() if pattern.search(v)]
+        else:
+            vars_ = fnmatch.filter(list(self.variables.keys()), pattern)
+
+        return vars_ if return_type == 'name' \
+            else [self.variables[v] for v in vars_]
 
 
 class BIDSRunVariableCollection(BIDSVariableCollection):
diff --git a/bids/variables/variables.py b/bids/variables/variables.py
index 3af6f1d1..78e0ea1a 100644
--- a/bids/variables/variables.py
+++ b/bids/variables/variables.py
@@ -279,22 +279,24 @@ class SimpleVariable(BIDSVariable):
 
         Parameters
         ----------
-        grouper : iterable
-            list to groupby, where each unique value will
-            be taken as the name of the resulting column.
+        grouper : :obj:`pandas.DataFrame`
+            Binary DF specifying the design matrix to use for splitting. Number
+            of rows must match current ``SparseRunVariable``;
+            a new ``SparseRunVariable`` will be generated for each column in
+            the grouper.
 
         Returns
         -------
-        A list of SparseRunVariables, one per unique value in the
-        grouper.
+        A list of SparseRunVariables, one per column in the grouper DF.
         """
         data = self.to_df(condition=True, entities=True)
         data = data.drop('condition', axis=1)
 
         subsets = []
-        for i, (name, g) in enumerate(data.groupby(grouper)):
-            name = '%s.%s' % (self.name, name)
-            col = self.__class__(name=name, data=g, source=self.source,
+        for i, col_name in enumerate(grouper.columns):
+            col_data = data.loc[grouper[col_name], :]
+            name = '{}.{}'.format(self.name, col_name)
+            col = self.__class__(name=name, data=col_data, source=self.source,
                                  run_info=getattr(self, 'run_info', None))
             subsets.append(col)
         return subsets


Test patch:
diff --git a/bids/analysis/tests/test_transformations.py b/bids/analysis/tests/test_transformations.py
index 9183ff2f..4cebd482 100644
--- a/bids/analysis/tests/test_transformations.py
+++ b/bids/analysis/tests/test_transformations.py
@@ -196,24 +196,33 @@ def test_split(collection):
         .clone(name='RT_3').to_dense(collection.sampling_rate)
 
     rt_pre_onsets = collection['RT'].onset
+    rt_pre_values = collection['RT'].values.values
 
     # Grouping SparseEventVariable by one column
     transform.Split(collection, ['RT'], ['respcat'])
-    assert 'RT.0' in collection.variables.keys() and \
-           'RT.-1' in collection.variables.keys()
-    rt_post_onsets = np.r_[collection['RT.0'].onset,
-                           collection['RT.-1'].onset,
-                           collection['RT.1'].onset]
+
+    # Verify names
+    assert 'RT.respcat[0]' in collection.variables.keys() and \
+           'RT.respcat[-1]' in collection.variables.keys()
+
+    # Verify values
+    rt_post_onsets = np.r_[collection['RT.respcat[0]'].onset,
+                           collection['RT.respcat[-1]'].onset,
+                           collection['RT.respcat[1]'].onset]
     assert np.array_equal(rt_pre_onsets.sort(), rt_post_onsets.sort())
 
+    rt_post_values = np.r_[collection['RT.respcat[0]'].values.values,
+                           collection['RT.respcat[-1]'].values.values,
+                           collection['RT.respcat[1]'].values.values]
+    assert np.array_equal(rt_pre_values.sort(), rt_post_values.sort())
+
     # Grouping SparseEventVariable by multiple columns
     transform.Split(collection, variables=['RT_2'], by=['respcat', 'loss'])
-    assert 'RT_2.-1_13' in collection.variables.keys() and \
-           'RT_2.1_13' in collection.variables.keys()
+    assert 'RT_2.respcat[-1].loss[13]' in collection.variables.keys() and \
+           'RT_2.respcat[1].loss[13]' in collection.variables.keys()
 
     # Grouping by DenseEventVariable
-    transform.Split(collection, variables='RT_3', by='respcat',
-                    drop_orig=False)
+    transform.Split(collection, variables='RT_3', by='respcat')
     targets = ['RT_3.respcat[-1]', 'RT_3.respcat[0]', 'RT_3.respcat[1]']
     assert not set(targets) - set(collection.variables.keys())
     assert collection['respcat'].values.nunique() == 3
@@ -223,7 +232,7 @@ def test_split(collection):
     # Grouping by entities in the index
     collection['RT_4'] = orig.clone(name='RT_4')
     transform.Split(collection, variables=['RT_4'], by=['respcat', 'run'])
-    assert 'RT_4.-1_3' in collection.variables.keys()
+    assert 'RT_4.respcat[-1].run[3]' in collection.variables.keys()
 
 
 def test_resample_dense(collection):
@@ -292,13 +301,12 @@ def test_copy(collection):
                           collection['RT_copy'].values.values)
 
 
-def test_regex_variable_expansion(collection):
+def test_expand_variable_names(collection):
     # Should fail because two output values are required following expansion
     with pytest.raises(Exception):
-        transform.Copy(collection, 'resp', regex_variables='variables')
+        transform.Copy(collection, '*resp*')
 
-    transform.Copy(collection, 'resp', output_suffix='_copy',
-                   regex_variables='variables')
+    transform.Copy(collection, '*resp*', output_suffix='_copy')
     assert 'respnum_copy' in collection.variables.keys()
     assert 'respcat_copy' in collection.variables.keys()
     assert np.array_equal(collection['respcat'].values.values,
@@ -453,3 +461,21 @@ def test_dropna(sparse_run_variable_with_missing_values):
     assert np.array_equal(post_trans.onset, [2, 5, 17])
     assert np.array_equal(post_trans.duration, [1.2, 1.6, 2])
     assert len(post_trans.index) == 3
+
+
+def test_group(collection):
+    coll = collection.clone()
+    with pytest.raises(ValueError):
+        # Can't use an existing variable name as the group name
+        transform.Group(coll, ['gain', 'loss'], name='gain')
+
+    transform.Group(coll, ['gain', 'loss'], name='outcome_vars')
+    assert coll.groups == { 'outcome_vars': ['gain', 'loss'] }
+
+    # Checks that variable groups are replaced properly
+    transform.Rename(coll, ['outcome_vars'],
+                     output=['gain_renamed', 'loss_renamed'])
+    assert 'gain_renamed' in coll.variables
+    assert 'loss_renamed' in coll.variables
+    assert 'gain' not in coll.variables
+    assert 'loss' not in coll.variables
diff --git a/bids/variables/tests/test_collections.py b/bids/variables/tests/test_collections.py
index 57786dc9..d5ee9509 100644
--- a/bids/variables/tests/test_collections.py
+++ b/bids/variables/tests/test_collections.py
@@ -2,7 +2,8 @@ from bids.layout import BIDSLayout
 import pytest
 from os.path import join, dirname, abspath
 from bids.tests import get_test_data_path
-from bids.variables import DenseRunVariable, merge_collections
+from bids.variables import (DenseRunVariable, SparseRunVariable,
+                            merge_collections)
 
 
 @pytest.fixture(scope="module")
@@ -104,3 +105,15 @@ def test_get_collection_entities(run_coll_list):
     ents = merged.entities
     assert {'task', 'subject', 'suffix', 'datatype'} == set(ents.keys())
     assert ents['subject'] == '02'
+
+
+def test_match_variables(run_coll):
+    matches = run_coll.match_variables('^.{1,2}a', match_type='regex')
+    assert set(matches) == {'gain', 'parametric gain'}
+    assert not run_coll.match_variables('.{1,3}a')
+    matches = run_coll.match_variables('^.{1,2}a', match_type='regex',
+                                       return_type='variable')
+    assert len(matches) == 2
+    assert all([isinstance(m, SparseRunVariable) for m in matches])
+    matches = run_coll.match_variables('*gain')
+    assert set(matches) == {'gain', 'parametric gain'}