Repository: bids-standard/pybids

Issue:
Transformation fails when output is different shape from inpus (even when its a new variable output)
A Neurosout user is attempting to apply an `And `operation.  There is an issue here because the two variabels are temporally unaligned (different shapes), so the `And` transformation doesn't make sense, but the error pybids spits back is about creating a new output column (not failing to do an `And` tranformation).

The transformation is (not that a new output column is specified)
```
   [{'Input': ['face_id_1', 'speech'],
     'Name': 'And',
     'Output': 'face_speech'},
```
and  the error is
```
/usr/local/lib/python3.6/site-packages/bids/analysis/transformations/base.py in __new__(cls, collection, variables, *args, **kwargs)
     86         t = super(Transformation, cls).__new__(cls)
     87         t._setup(collection, variables, *args, **kwargs)
---> 88         return t.transform()
     89 
     90     def _setup(self, collection, variables, *args, **kwargs):

/usr/local/lib/python3.6/site-packages/bids/analysis/transformations/base.py in transform(self)
    253                 result = self._transform(data, **kwargs)
    254                 if self._return_type not in ['none', None]:
--> 255                     col = col[0].clone(data=result, name=self.output[0])
    256             # Otherwise loop over variables individually
    257             else:

/usr/local/lib/python3.6/site-packages/bids/variables/variables.py in clone(self, data, **kwargs)
     47                 raise ValueError("Replacement data has shape %s; must have "
     48                                  "same shape as existing data %s." %
---> 49                                  (data.shape, self.values.shape))
     50             result.values = pd.DataFrame(data)
     51 

ValueError: Replacement data has shape (495,); must have same shape as existing data (418,).
```

Looks like what's happening is that the resulting data is 495 rows, but pybids uses `.clone` to create the new output column, and this only works if the new data is the same shape as the input.

Issues with the `And` transformation on unaligned variables aligned, should it be possible for a transformation to result in different shaped results, especially if a new column name is specified?

Or would this only occur in this scenario when two variables are unaligned (in which case, maybe we should catch this situation earlier0>

Discussion:

Patch:
diff --git a/bids/analysis/transformations/base.py b/bids/analysis/transformations/base.py
index b1e1e1ea..45480171 100644
--- a/bids/analysis/transformations/base.py
+++ b/bids/analysis/transformations/base.py
@@ -41,10 +41,18 @@ class Transformation(metaclass=ABCMeta):
     # no further changes will be committed.
     _return_type = 'pandas'
 
-    # A tuple indicating which arguments give the names of variables that must
-    # all be aligned with one another (i.e., onsets and durations match
-    # perfectly) before processing. Defaults to None.
-    _align = None
+    # Indicates if variables must be aligned with one another
+    # (i.e., onsets and durations match perfectly) before processing.
+    # If True, will require all variables to be aligned (throw exception).
+    # If 'force', will auto-align variables by densification.
+    # Defaults to None.
+    _aligned_required = None
+
+    # (Optional) If alignment is required,sets which variables must be aligned.
+    # If None, all variables must be aligned. It can also be a a string
+    # indicating which column contains the names of columns to be aligned,
+    # or a tuple which explicitly names the columns to be aligned.
+    _aligned_variables = None
 
     # Boolean indicating whether the Transformation should be applied to each
     # variable in the input list in turn. When True (default), Transformation
@@ -311,7 +319,7 @@ class Transformation(metaclass=ABCMeta):
     def _postprocess(self, col):
         return col
 
-    def _align_variables(self, variables, force=True):
+    def _align_variables(self, variables):
         """Checks whether the specified variables have aligned indexes. This
         implies either that all variables are dense, or that all variables are
         sparse and have exactly the same onsets and durations. If variables are
@@ -319,7 +327,7 @@ class Transformation(metaclass=ABCMeta):
         format in order to ensure alignment.
         """
 
-        if self._align is None or self._align == 'none':
+        if self._aligned_required is None or self._aligned_required == 'none':
             return
 
         def _align(variables):
@@ -328,15 +336,8 @@ class Transformation(metaclass=ABCMeta):
                       if isinstance(c, SparseRunVariable)]
             if len(sparse) < len(variables):
                 if sparse:
-                    sparse_names = [s.name for s in sparse]
                     msg = ("Found a mix of dense and sparse variables. May "
                            "cause problems for some transformations.")
-                    if force:
-                        msg += (" Sparse variables %s were converted to dense "
-                                "form to ensure proper alignment." %
-                                sparse_names)
-                        sr = self.collection.sampling_rate
-                        sparse = [s.to_dense(sr) for s in sparse]
                     warnings.warn(msg)
             # If all are sparse, durations, onsets, and index must match
             # perfectly for all
@@ -351,28 +352,37 @@ class Transformation(metaclass=ABCMeta):
                 fc = get_col_data(variables[0])
                 if not all([compare_variables(fc, get_col_data(c))
                             for c in variables[1:]]):
-                    msg = "Misaligned sparse variables found."
-                    if force:
-                        msg += (" Forcing all sparse variables to dense in "
-                                "order to ensure proper alignment.")
+                    if self._aligned_required == 'force_dense':
+                        msg = ("Forcing all sparse variables to dense in "
+                               "order to ensure proper alignment.")
                         sr = self.collection.sampling_rate
                         variables = [c.to_dense(sr) for c in variables]
-                    warnings.warn(msg)
-
-        align_variables = [listify(self.kwargs[v])
-                           for v in listify(self._align) if v in self.kwargs]
-        align_variables = list(itertools.chain(*align_variables))
-        align_variables = [self.collection[c] for c in align_variables if c]
-
-        if align_variables and self._loopable:
+                        warnings.warn(msg)
+                    else:
+                        raise ValueError(
+                            "Misaligned sparse variables found."
+                            "To force variables into alignment by densifying,"
+                            "set dense=True in the Transformation arguments"
+                            )
+
+        _aligned_variables = True if not self._aligned_variables \
+            else self._aligned_variables
+        _aligned_variables = [listify(self.kwargs[v])
+                              for v in listify(_aligned_variables)
+                              if v in self.kwargs]
+        _aligned_variables = list(itertools.chain(*_aligned_variables))
+        _aligned_variables = [
+            self.collection[c] for c in _aligned_variables if c]
+
+        if _aligned_variables and self._loopable:
             for c in variables:
                 # TODO: should clone all variables in align_variables before
                 # alignment to prevent conversion to dense in any given
                 # iteration having side effects. This could be an issue if,
                 # e.g., some vars in 'variables' are dense and some are sparse.
-                _align([c] + align_variables)
+                _align([c] + _aligned_variables)
         else:
-            _align(listify(variables) + align_variables)
+            _align(listify(variables) + _aligned_variables)
 
 
 class TransformerManager(object):
diff --git a/bids/analysis/transformations/compute.py b/bids/analysis/transformations/compute.py
index a3e97e63..b9a151f8 100644
--- a/bids/analysis/transformations/compute.py
+++ b/bids/analysis/transformations/compute.py
@@ -107,7 +107,8 @@ class Orthogonalize(Transformation):
 
     _variables_used = ('variables', 'other')
     _densify = ('variables', 'other')
-    _align = ('other')
+    _aligned_required = 'force_dense'
+    _aligned_variables = ('other')
 
     def _transform(self, var, other):
 
@@ -129,7 +130,7 @@ class Product(Transformation):
 
     _loopable = False
     _groupable = False
-    _align = True
+    _aligned_required = True
     _output_required = True
 
     def _transform(self, data):
@@ -181,7 +182,7 @@ class Sum(Transformation):
 
     _loopable = False
     _groupable = False
-    _align = True
+    _aligned_required = True
     _output_required = True
 
     def _transform(self, data, weights=None):
@@ -197,7 +198,6 @@ class Sum(Transformation):
         return (data * weights).sum(axis=1)
 
 
-
 class Threshold(Transformation):
     """Threshold and/or binarize a variable.
 
@@ -250,6 +250,7 @@ class And_(Transformation):
     _loopable = False
     _groupable = False
     _output_required = True
+    _aligned_required = True
 
     def _transform(self, dfs):
         df = pd.concat(dfs, axis=1, sort=True)
@@ -284,6 +285,7 @@ class Or_(Transformation):
     _loopable = False
     _groupable = False
     _output_required = True
+    _aligned_required = True
 
     def _transform(self, dfs):
         df = pd.concat(dfs, axis=1, sort=True)
diff --git a/bids/analysis/transformations/munge.py b/bids/analysis/transformations/munge.py
index 38c4c7d6..2e357e70 100644
--- a/bids/analysis/transformations/munge.py
+++ b/bids/analysis/transformations/munge.py
@@ -175,7 +175,8 @@ class Filter(Transformation):
     _groupable = False
     _input_type = 'variable'
     _return_type = 'variable'
-    _align = ('by')
+    _aligned_required = 'force_dense'
+    _aligned_variables = ('by')
     _allow_categorical = ('variables', 'by')
 
     def _transform(self, var, query, by=None):
diff --git a/bids/variables/variables.py b/bids/variables/variables.py
index b4afc9f2..4b6fc60c 100644
--- a/bids/variables/variables.py
+++ b/bids/variables/variables.py
@@ -46,9 +46,13 @@ class BIDSVariable(metaclass=ABCMeta):
         result = deepcopy(self)
         if data is not None:
             if data.shape != self.values.shape:
-                raise ValueError("Replacement data has shape %s; must have "
-                                 "same shape as existing data %s." %
-                                 (data.shape, self.values.shape))
+                # If data can be re-shaped safely, do so
+                if data.squeeze().shape == self.values.squeeze().shape:
+                    data = data.values.reshape(self.values.shape)
+                else:
+                    raise ValueError("Replacement data has shape %s; must have"
+                                     " same shape as existing data %s." %
+                                     (data.shape, self.values.shape))
             result.values = pd.DataFrame(data)
 
         if kwargs:


Test patch:
diff --git a/bids/analysis/tests/test_transformations.py b/bids/analysis/tests/test_transformations.py
index 6545d324..93aa2969 100644
--- a/bids/analysis/tests/test_transformations.py
+++ b/bids/analysis/tests/test_transformations.py
@@ -461,13 +461,15 @@ def test_and(collection):
     coll = collection.clone()
     transform.Factor(coll, 'respnum')
     names = ['respnum.%d' % d for d in range(0, 5)]
-    transform.And(coll, names, output='conjunction')
-    assert not coll.variables['conjunction'].values.sum()
 
-    coll['copy'] = coll.variables['respnum.0'].clone()
-    transform.And(coll, ['respnum.0', 'copy'], output='conj')
-    assert coll.variables['conj'].values.astype(float).equals(
-        coll.variables['respnum.0'].values)
+    coll.variables['respnum.0'].onset += 1
+
+    # Should fail because I misaligned variable
+    with pytest.raises(ValueError):
+        transform.And(coll, names, output='misaligned')
+
+    # Should pass because dense is set to True and will align
+    transform.And(coll, names, output='misaligned', dense=True)
 
 
 def test_or(collection):