Merge remote-tracking branch 'upstream/master' into fix/matching-paths

Author: effigies

.github/workflows/codespell.yml

@@ -13,5 +13,5 @@ jobs:
     runs-on: ubuntu-latest
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
       - uses: codespell-project/actions-codespell@master

.github/workflows/docs.yml

@@ -22,12 +22,12 @@ jobs:
         include:
           - python-version: 3.9
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           submodules: recursive
           fetch-depth: 0
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v3
         with:
           python-version: ${{ matrix.python-version }}
       - name: Display Python version
@@ -44,7 +44,7 @@ jobs:
             source tools/ci/activate.sh
             make -C doc html
       - name: Upload docs as artifacts
-        uses: actions/upload-artifact@v2
+        uses: actions/upload-artifact@v3
         with:
             path: doc/_build/html
       - name: Deploy (on tags)

.github/workflows/package.yml

@@ -23,12 +23,12 @@ jobs:
           - os: ubuntu-latest
             python-version: "3.10"
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           submodules: recursive
           fetch-depth: 0
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v3
         with:
           python-version: ${{ matrix.python-version }}
       - name: Display Python version

.github/workflows/pre-release.yml

@@ -38,14 +38,14 @@ jobs:
       OS_TYPE: ${{ matrix.os }}
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           submodules: recursive
           fetch-depth: 0
       - name: Install dependencies
         run: tools/ci/install_dependencies.sh
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v3
         with:
           python-version: ${{ matrix.python-version }}
       - name: Display Python version

.github/workflows/stable.yml

@@ -70,14 +70,14 @@ jobs:
       OS_TYPE: ${{ matrix.os }}
 
     steps:
-      - uses: actions/checkout@v2
+      - uses: actions/checkout@v3
         with:
           submodules: recursive
           fetch-depth: 0
       - name: Install dependencies
         run: tools/ci/install_dependencies.sh
       - name: Set up Python ${{ matrix.python-version }}
-        uses: actions/setup-python@v2
+        uses: actions/setup-python@v3
         with:
           python-version: ${{ matrix.python-version }}
       - name: Display Python version

bids/layout/config/bids.json

@@ -46,3 +46,92 @@ def test_insufficient_entities(layout, strict, validate):
     """Check https://github.com/bids-standard/pybids/pull/574#discussion_r366447600."""
     with pytest.raises(ValueError):
         layout.build_path({'subject': '01'}, strict=strict, validate=validate)
+
+
+"""
+The following tests that indexes some datasets from bids examples, 
+parses the files for entities and 
+reconstructs the fullpath of each file by relying on pybids config 
+and compares it what's actually in the dataset
+"""
+
+@pytest.mark.parametrize("scope", ["raw"])
+@pytest.mark.parametrize(
+    "dataset",
+    [
+        ("qmri_irt1"),
+        ("qmri_mese"),
+        ("qmri_mp2rage"),
+        ("qmri_mp2rageme"),
+        ("qmri_mtsat"),
+        ("qmri_sa2rage"),
+        ("qmri_vfa"),
+        ("ds000117"),
+    ],
+)
+def test_path_building_on_examples_with_derivatives(dataset, scope, bids_examples):
+    layout = BIDSLayout(bids_examples / dataset, derivatives=True)
+    files = layout.get(subject=".*", datatype=".*", regex_search = True, scope=scope)
+    for bf in files:
+        entities = bf.get_entities()
+        path = layout.build_path(entities)
+        assert(path==bf.path)
+
+@pytest.mark.parametrize(
+    "dataset",
+    [
+        ("micr_SEM"),
+        ("micr_SPIM"),
+        ("asl001"),
+        ("asl002"),
+        ("asl003"),
+        ("asl004"),
+        ("asl005"),
+        ("pet001"),
+        ("pet002"),
+        ("pet003"),
+        ("pet004"),
+        ("pet005"),
+        ("qmri_megre"),
+        ("qmri_tb1tfl"),
+        ("eeg_cbm"),
+        ("ieeg_filtered_speech"),
+        ("ieeg_visual_multimodal"),
+        ("ds000248"),
+        ("ds001"),
+        ("ds114"),
+    ],
+)
+def test_path_building_on_examples_with_no_derivatives(dataset, bids_examples):
+    layout = BIDSLayout(bids_examples / dataset, derivatives=False)
+    files = layout.get(subject=".*", datatype=".*", regex_search =  True)
+    for bf in files:
+        entities = bf.get_entities()
+        path = layout.build_path(entities)
+        assert(path==bf.path)
+
+@pytest.mark.parametrize("scope", ["raw"])
+@pytest.mark.parametrize(
+    "dataset",
+    [
+        pytest.param(
+            "ds000247",
+            marks=pytest.mark.xfail(strict=True,
+                reason="meg ds folder"
+            ),
+        ),
+        pytest.param(
+            "ds000246",
+            marks=pytest.mark.xfail(strict=True,
+                reason="meg ds folder"
+            ),
+        ),
+    ],
+)
+def test_path_building_on_examples_with_derivatives_meg_ds_folder(dataset, scope, bids_examples):
+    layout = BIDSLayout(bids_examples / dataset, derivatives=True)
+    files = layout.get(subject=".*", datatype=".*", regex_search = True, scope=scope)
+    for bf in files:
+        entities = bf.get_entities()
+        path = layout.build_path(entities)
+        assert(path==bf.path)

bids/layout/tests/test_path_building.py

@@ -4,6 +4,7 @@
 from collections import namedtuple, OrderedDict, Counter, defaultdict
 import itertools
 from functools import reduce
+from multiprocessing.sharedctypes import Value
 import re
 import fnmatch
 
@@ -263,11 +264,11 @@ class BIDSStatsModelsNode:
         overridden if one is passed when run() is called on a node.
     """
 
-    def __init__(self, level, name, transformations=None, model=None,
-                 contrasts=None, dummy_contrasts=False, group_by=None):
+    def __init__(self, level, name, model, group_by, transformations=None,
+                 contrasts=None, dummy_contrasts=False):
         self.level = level.lower()
         self.name = name
-        self.model = model or {}
+        self.model = model
         if transformations is None:
             transformations = {"transformer": "pybids-transforms-v1",
                                "instructions": []}
@@ -279,13 +280,7 @@ def __init__(self, level, name, transformations=None, model=None,
         self.children = []
         self.parents = []
         if group_by is None:
-            group_by = []
-            # Loop over contrasts after first level
-            if self.level != "run":
-                group_by.append("contrast")
-            # Loop over node level of this node
-            if self.level != "dataset":
-                group_by.append(self.level)
+            raise ValueError(f"group_by is not defined for Node: {name}")
         self.group_by = group_by
 
         # Check for intercept only run level model and throw an error
@@ -592,25 +587,21 @@ def __init__(self, node, entities={}, collections=None, inputs=None,
 
         var_names = list(self.node.model['x'])
 
-        # Handle the special 1 construct. If it's present, we add a
-        # column of 1's to the design matrix. But behavior varies:
-        # * If there's only a single contrast across all of the inputs,
-        #   the intercept column is given the same name as the input contrast.
-        #   It may already exist, in which case we do nothing.
-        # * Otherwise, we name the column 'intercept'.
-        int_name = None
+        # Handle the special 1 construct.
+        # Add column of 1's to the design matrix called "intercept" 
         if 1 in var_names:
-            if ('contrast' not in df.columns or df['contrast'].nunique() > 1):
-                int_name = 'intercept'
-            else:
-                int_name = df['contrast'].unique()[0]
-
-            var_names.remove(1)
-
-            if int_name not in df.columns:
-                df.insert(0, int_name, 1)
-            else:
-                var_names.append(int_name)
+            if "intercept" in var_names:
+                raise ValueError("Cannot define both '1' and 'intercept' in 'X'")
+                
+            var_names = ['intercept' if i == 1 else i for i in var_names]
+            if 'intercept' not in df.columns:
+                df.insert(0, 'intercept', 1)
+
+        # If a single incoming contrast
+        if ('contrast' in df.columns and df['contrast'].nunique() == 1):
+            unique_in_contrast = df['contrast'].unique()[0]
+        else:
+            unique_in_contrast = None
 
         var_names = expand_wildcards(var_names, df.columns)
 
@@ -626,7 +617,7 @@ def __init__(self, node, entities={}, collections=None, inputs=None,
 
         # Create ModelSpec and build contrasts
         self.model_spec = create_model_spec(self.data, node.model, self.metadata)
-        self.contrasts = self._build_contrasts(int_name)
+        self.contrasts = self._build_contrasts(unique_in_contrast)
 
     def _collections_to_dfs(self, collections):
         """Merges collections and converts them to a pandas DataFrame."""
@@ -690,17 +681,58 @@ def _inputs_to_df(self, inputs):
                 input_df.loc[input_df.index[i], con.name] = 1
         return input_df
 
-    def _build_contrasts(self, int_name):
-        """Contrast list of ContrastInfo objects based on current state."""
-        contrasts = {}
+    def _build_contrasts(self, unique_in_contrast=None):
+        """Contrast list of ContrastInfo objects based on current state.
+        
+        Parameters
+        ----------
+        unique_in_contrast : string
+            Name of unique incoming contrast inputs (i.e. if there is only 1)
+        """
+        in_contrasts = self.node.contrasts.copy()
         col_names = set(self.X.columns)
-        for con in self.node.contrasts:
-            name = con["name"]
+
+        # Create dummy contrasts as regular contrasts
+        dummies = self.node.dummy_contrasts
+        if dummies:
+            if 'conditionlist' in dummies:
+                conditions = set(dummies['condition_list'])
+            else:
+                conditions = col_names
+
+            for col_name in conditions:
+                if col_name == "intercept":
+                    col_name = 1
+
+                in_contrasts.insert(0, 
+                    {
+                        'name': col_name,
+                        'condition_list': [col_name],
+                        'weights': [1],
+                        'test': dummies.get('test')
+                    }
+                )
+
+        # Process all contrasts, starting with dummy contrasts 
+        # Dummy contrasts are replaced if a contrast is defined with same name
+        contrasts = {}
+        for con in in_contrasts:
             condition_list = list(con["condition_list"])
-            if 1 in condition_list and int_name is not None:
-                condition_list[condition_list.index(1)] = int_name
-            if name == 1 and int_name is not None:
-                name = int_name
+
+            # Rename special 1 construct
+            condition_list = ['intercept' if i == 1 else i for i in condition_list]
+
+            name = con["name"]
+            
+            # Rename contrast name
+            if name == 1:
+                name = unique_in_contrast or 'intercept'
+            else:
+                # If Node has single contrast input, as is grouped by contrast
+                # Rename contrast to append incoming contrast name
+                if unique_in_contrast:
+                    name = f"{unique_in_contrast}_{name}" 
+                    
             missing_vars = set(condition_list) - col_names
             if missing_vars:
                 if self.invalid_contrasts == 'error':
@@ -711,31 +743,13 @@ def _build_contrasts(self, int_name):
                 elif self.invalid_contrasts == 'drop':
                     continue
             weights = np.atleast_2d(con['weights'])
+
             # Add contrast name to entities; can be used in grouping downstream
             entities = {**self.entities, 'contrast': name}
             ci = ContrastInfo(name, condition_list,
                               con['weights'], con.get("test"), entities)
             contrasts[name] = ci
 
-        dummies = self.node.dummy_contrasts
-        if dummies:
-            conditions = col_names
-            if 'conditions' in dummies:
-                conds = set(dummies['conditions'])
-                if 1 in conds and int_name is not None:
-                    conds.discard(1)
-                    conds.add(int_name)
-                conditions &= conds
-            conditions -= set(c.name for c in contrasts.values())
-
-            for col_name in conditions:
-                if col_name in contrasts:
-                    continue
-                entities = {**self.entities, 'contrast': col_name}
-                ci = ContrastInfo(col_name, [col_name], [1], dummies.get("test"),
-                                  entities)
-                contrasts[col_name] = ci
-
         return list(contrasts.values())
 
     @property