[ENH] do not add subject to layout if it is empty (#600)

* do not add subject to layout if it is empty

* subject is in structure if schemaless

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* refactor

* get properties and not fields when dealing with bids.File instance

* rerun

* silence failing tests on windows

---------

Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>

+bids/+internal/keep_file_for_query.m

@@ -59,10 +59,16 @@
 
 end
 
-function  status = check(status, structure, key, values)
+function  status = check(status, file_struct, key, values)
 
   % does the file have the entity or does the filename structure has this fieldname ?
-  has_key = ismember(key, fieldnames(structure));
+  if isstruct(file_struct)
+    fields = fieldnames(file_struct);
+  elseif strcmp(class(file_struct), 'bids.File')
+    fields = properties(file_struct);
+  end
+
+  has_key = ismember(key, fields);
   % do we want to exclude the file (by passing an empty option) bassed on that key ?
   exclude = numel(values) == 1 && isempty(values{1});
 
@@ -75,7 +81,7 @@
     return
   end
 
-  value = structure.(key);
+  value = file_struct.(key);
 
   if has_key && exclude && ...
           ~isempty(value)

+bids/layout.m

@@ -99,14 +99,11 @@
 
   if ischar(root)
     root = bids.internal.file_utils(root, 'CPath');
-
   elseif isstruct(root)
     BIDS = root; % for bids.query
     return
-
   else
     error('Invalid syntax.');
-
   end
 
   if verbose
@@ -144,19 +141,7 @@
   BIDS.participants = [];
   BIDS.participants = manage_tsv(BIDS.participants, BIDS.pth, 'participants.tsv', verbose);
 
-  BIDS.phenotype = struct('file', [], 'metafile', []);
-
-  assessments = bids.internal.file_utils('FPList', ...
-                                         fullfile(BIDS.pth, 'phenotype'), ...
-                                         '.*\.tsv$');
-  for i = 1:size(assessments, 1)
-    file = deblank(assessments(i, :));
-    sidecar = bids.internal.file_utils(file, 'ext', 'json');
-    BIDS.phenotype(i).file = file;
-    if exist(sidecar, 'file') == 2
-      BIDS.phenotype(i).metafile = sidecar;
-    end
-  end
+  BIDS = index_phenotype(BIDS);
 
   BIDS = index_root_directory(BIDS);
 
@@ -184,6 +169,16 @@
       continue
     end
 
+    if use_schema && isempty(ls(fullfile(BIDS.pth, subjects{iSub})))
+      if verbose
+        msg = sprintf('subject ''%s'' is empty. Skipping.', ...
+                      subjects{iSub});
+        bids.internal.error_handling(mfilename, 'EmptySubject', ...
+                                     msg, tolerant, verbose);
+      end
+      continue
+    end
+
     if verbose
       fprintf(1, ' Indexing subject: %s [', subjects{iSub});
     end
@@ -248,6 +243,22 @@ function handle_invalid_input(ME, root)
   end
 end
 
+function BIDS = index_phenotype(BIDS)
+  BIDS.phenotype = struct('file', [], 'metafile', []);
+
+  assessments = bids.internal.file_utils('FPList', ...
+                                         fullfile(BIDS.pth, 'phenotype'), ...
+                                         '.*\.tsv$');
+  for i = 1:size(assessments, 1)
+    file = deblank(assessments(i, :));
+    sidecar = bids.internal.file_utils(file, 'ext', 'json');
+    BIDS.phenotype(i).file = file;
+    if exist(sidecar, 'file') == 2
+      BIDS.phenotype(i).metafile = sidecar;
+    end
+  end
+end
+
 function value = exclude(filter, entity, label)
   value = false;
   % skip if not included in filter

tests/tests_layout/test_layout.m

@@ -6,6 +6,49 @@
   initTestSuite;
 end
 
+function test_layout_do_not_include_empty_subject()
+
+  if ispc
+    % TODO investigate
+    moxunit_throw_test_skipped_exception('fail on windows');
+  end
+
+  bids_dir = fullfile(get_test_data_dir(), 'qmri_tb1tfl');
+  empty_sub = fullfile(bids_dir, 'sub-02');
+  bids.util.mkdir(fullfile(bids_dir, 'sub-02'));
+
+  verbose = false;
+
+  BIDS = bids.layout(bids_dir, 'verbose', verbose, 'use_schema', false);
+  assertEqual(numel(bids.query(BIDS, 'subjects')), 1);
+  assertEqual(numel(BIDS.subjects), 2);
+
+  BIDS = bids.layout(bids_dir, 'verbose', verbose);
+  assertEqual(numel(bids.query(BIDS, 'subjects')), 1);
+  assertEqual(numel(BIDS.subjects), 1);
+
+  rmdir(empty_sub);
+
+end
+
+function test_layout_do_not_include_empty_subject_warning()
+
+  if bids.internal.is_octave() || ispc
+    moxunit_throw_test_skipped_exception('Octave mixed-string-concat or fail on windows');
+  end
+
+  bids_dir = fullfile(get_test_data_dir(), 'qmri_tb1tfl');
+  empty_sub = fullfile(bids_dir, 'sub-02');
+  bids.util.mkdir(fullfile(bids_dir, 'sub-02'));
+
+  verbose = true;
+  assertWarning(@()bids.layout(bids_dir, 'verbose', verbose), ...
+                'layout:EmptySubject');
+
+  rmdir(empty_sub);
+
+end
+
 function test_layout_error_message
 
   assertExceptionThrown(@()bids.layout('foo'), 'layout:InvalidInput');