Merge pull request #97 from dartmouth-pbs/sort-scans

Sort scans, .heudiconv into subdataset in --datalad mode

Author: satra

bin/heudiconv

@@ -49,6 +49,10 @@ from os.path import join as pjoin
 
 from random import sample
 
+# Minimal versions of external dependencies
+MIN_VERSIONS = {
+    'datalad': '0.7'
+}
 PY3 = sys.version_info[0] >= 3
 
 import logging
@@ -1024,11 +1028,13 @@ def add_rows_to_scans_keys_file(fn, newrows):
 
     header = ['filename', 'acq_time', 'operator', 'randstr']
     # save
+    # prepare all the data rows
+    data_rows = [[k] + v for k, v in fnames2info.items()]
+    # sort by the date/filename
+    data_rows_sorted = sorted(data_rows, key=lambda x: (x[1], x[0]))
     with open(fn, 'a') as csvfile:
         writer = csv.writer(csvfile, delimiter='\t')
-        writer.writerow(header)
-        for key in sorted(fnames2info.keys()):
-            writer.writerow([key] + fnames2info[key])
+        writer.writerows([header] + data_rows_sorted)
 
 
 def _find_subj_ses(f_name):
@@ -1561,6 +1567,9 @@ def create_file_if_missing(filename, content):
     """Create file if missing, so we do not override any possibly introduced changes"""
     if exists(filename):
         return False
+    dirname = os.path.dirname(filename)
+    if not os.path.exists(dirname):
+        os.makedirs(dirname)
     with open(filename, 'w') as f:
         f.write(content)
     return True
@@ -1697,7 +1706,7 @@ def mark_sensitive(ds, path_glob=None):
         if not paths:
             return
         sens_kwargs['path'] = paths
-    ds.metadata(**sens_kwargs)
+    ds.metadata(recursive=True, **sens_kwargs)
 
 
 def add_to_datalad(topdir, studydir, msg=None, bids=False):
@@ -1708,7 +1717,8 @@ def add_to_datalad(topdir, studydir, msg=None, bids=False):
     from datalad.support.annexrepo import AnnexRepo
 
     from datalad.support.external_versions import external_versions
-    assert external_versions['datalad'] >= '0.5.1', "Need datalad >= 0.5.1"
+    # 0.7 added .metadata
+    assert external_versions['datalad'] >= MIN_VERSIONS['datalad'], "Need datalad >= 0.7"
 
     studyrelpath = os.path.relpath(studydir, topdir)
     assert not studyrelpath.startswith(os.path.pardir)  # so we are under
@@ -1753,21 +1763,37 @@ def add_to_datalad(topdir, studydir, msg=None, bids=False):
     ds = Dataset(studydir)
     # Add doesn't have all the options of save such as msg and supers
     ds.add('.gitattributes', to_git=True, save=False)
-    dsh = None
+    dsh = dsh_path = None
     if os.path.lexists(os.path.join(ds.path, '.heudiconv')):
-        dsh = Dataset(opj(ds.path, '.heudiconv'))
+        dsh_path = opj(ds.path, '.heudiconv')
+        dsh = Dataset(dsh_path)
         if not dsh.is_installed():
-            # we need to create it first
-            dsh = ds.create(path='.heudiconv',
-                            force=True,
-                            shared_access='all')
+            # Previously we did not have it as a submodule, and since no
+            # automagic migration is implemented, we just need to check first
+            # if any path under .heudiconv is already under git control
+            if any(x[0].startswith('.heudiconv/') for x in
+                   ds.repo.repo.index.entries.keys()):
+                lgr.warning(
+                    '%s has .heudiconv not as a submodule from previous versions '
+                    'of heudiconv.  No automagic migration is yet provided', ds
+                )
+            else:
+                # use/create a submodule dataset for .heudiconv
+                dsh = ds.create(path='.heudiconv',
+                                force=True,
+                                shared_access='all')
         # Since .heudiconv could contain sensitive information
         # we place all files under annex and then add
         if create_file_if_missing(
-            opj(dsh.path, '.gitattributes'),
+            opj(dsh_path, '.gitattributes'),
             """* annex.largefiles=anything
             """):
-            dsh.add('.gitattributes', message="Added gitattributes to place all content under annex")
+            # should work properly if .heudiconv is a submodule or not
+            ds.add(
+                '.heudiconv/.gitattributes',
+                to_git=True,
+                message="Added gitattributes to place all .heudiconv content under annex"
+            )
     ds.add('.', recursive=True, save=False,
            # not in effect! ?
            #annex_add_opts=['--include-dotfiles']
@@ -1781,9 +1807,10 @@ def add_to_datalad(topdir, studydir, msg=None, bids=False):
     mark_sensitive(ds, '*/*/*_scans.tsv')  # within sess/subj
     mark_sensitive(ds, '*/anat')  # within subj
     mark_sensitive(ds, '*/*/anat')  # within ses/subj
-    if dsh:
-        mark_sensitive(dsh)  # entire .heudiconv!
-        dsh.save(message=msg)
+    if dsh_path:
+        mark_sensitive(ds, '.heudiconv')  # entire .heudiconv!
+    # if dsh and dsh.is_installed():
+    #     dsh.save(message=msg)
     ds.save(message=msg, recursive=True, super_datasets=True)
 
     assert not ds.repo.dirty

heuristics/dbic_bids_validator.cfg

@@ -6,8 +6,16 @@
     "error": [],
     "ignoredFiles": [
           "/.heudiconv/*", "/.heudiconv/*/*", "/.heudiconv/*/*/*", "/.heudiconv/*/*/*/*",  
+          "/.heudiconv/.git*", 
+          "/.heudiconv/.git/*",
+          "/.heudiconv/.git/*/*",
+          "/.heudiconv/.git/*/*/*",
+          "/.heudiconv/.git/*/*/*/*",
+          "/.heudiconv/.git/*/*/*/*/*",
+          "/.heudiconv/.git/*/*/*/*/*/*",
           "/.git*", 
           "/.datalad/*", "/.datalad/.*", 
+          "/.*/.datalad/*", "/.*/.datalad/.*", 
           "/sub*/ses*/*/*__dup*", "/sub*/*/*__dup*"
     ]
 }

tests/test_main.py

@@ -67,7 +67,7 @@ def test_add_participant_record(tmpdir):
 
 
 def test_prepare_for_datalad(tmpdir):
-    pytest.importorskip("datalad")
+    pytest.importorskip("datalad", minversion=heudiconv.MIN_VERSIONS['datalad'])
     studydir = tmpdir.join("PI").join("study")
     studydir_ = str(studydir)
     os.makedirs(studydir_)
@@ -80,7 +80,7 @@ def test_prepare_for_datalad(tmpdir):
 
     assert superds.is_installed()
     assert not superds.repo.dirty
-    subdss = superds.get_subdatasets(recursive=True)
+    subdss = superds.subdatasets(recursive=True, result_xfm='relpaths')
     for ds_path in sorted(subdss):
         ds = Dataset(opj(superds.path, ds_path))
         assert ds.is_installed()
@@ -98,6 +98,35 @@ def test_prepare_for_datalad(tmpdir):
         assert not ds.repo.is_under_annex(f)
     assert not ds.repo.is_under_annex('.gitattributes')
 
+    # Above call to add_to_datalad does not create .heudiconv subds since
+    # directory does not exist (yet).
+    # Let's first check that it is safe to call it again
+    heudiconv.add_to_datalad(str(tmpdir), studydir_)
+    assert not ds.repo.dirty
+
+    old_hexsha = ds.repo.get_hexsha()
+    # Now let's check that if we had previously converted data so that
+    # .heudiconv was not a submodule, we still would not fail
+    dsh_path = os.path.join(ds.path, '.heudiconv')
+    dummy_path = os.path.join(dsh_path, 'dummy.nii.gz')
+
+    heudiconv.create_file_if_missing(dummy_path, '')
+    ds.add(dummy_path, message="added a dummy file")
+    # next call must not fail, should just issue a warning
+    heudiconv.add_to_datalad(str(tmpdir), studydir_)
+    ds.repo.is_under_annex(dummy_path)
+    assert not ds.repo.dirty
+    assert '.heudiconv/dummy.nii.gz' in ds.repo.get_files()
+
+    # Let's now roll back and make it a proper submodule
+    ds.repo._git_custom_command([], ['git', 'reset', '--hard', old_hexsha])
+    # now we do not add dummy to git
+    heudiconv.create_file_if_missing(dummy_path, '')
+    heudiconv.add_to_datalad(str(tmpdir), studydir_)
+    assert '.heudiconv' in ds.subdatasets(result_xfm='relpaths')
+    assert not ds.repo.dirty
+    assert '.heudiconv/dummy.nii.gz' not in ds.repo.get_files()
+
 
 def test_json_dumps_pretty():
     pretty = heudiconv.json_dumps_pretty
@@ -146,6 +175,9 @@ def _check_rows(fn, rows):
                 assert(row_ == ['filename', 'acq_time', 'operator', 'randstr'])
             else:
                 assert(rows[row_[0]] == row_[1:])
+        # dates, filename should be sorted (date "first", filename "second")
+        dates = [(r[1], r[0]) for r in rows_loaded[1:]]
+        assert dates == sorted(dates)
 
     _check_rows(fn, rows)
     # add a new one
@@ -157,6 +189,7 @@ def _check_rows(fn, rows):
     heudiconv.add_rows_to_scans_keys_file(fn, extra_rows)
     _check_rows(fn, extra_rows)
 
+
 def test__find_subj_ses():
     assert heudiconv._find_subj_ses('950_bids_test4/sub-phantom1sid1/fmap/sub-phantom1sid1_acq-3mm_phasediff.json') == ('phantom1sid1', None)
     assert heudiconv._find_subj_ses('sub-s1/ses-s1/fmap/sub-s1_ses-s1_acq-3mm_phasediff.json') == ('s1', 's1')