Merge pull request #425 from dbic/reproin-catchallremap

ReproIn: make specification of protocols2fix more flexible + do not bind fixup structs in func signatures

Author: yarikoptic

heudiconv/heuristics/reproin.py

@@ -126,6 +126,10 @@
 import logging
 lgr = logging.getLogger('heudiconv')
 
+# pythons before 3.7 didn't have re.Pattern, it was some protected
+# _sre.SRE_Pattern, so let's just sample a class of the compiled regex
+re_Pattern = re.compile('.').__class__
+
 # Terminology to harmonise and use to name variables etc
 # experiment
 #  subject
@@ -372,14 +376,14 @@ def get_study_hash(seqinfo):
     return md5sum(get_study_description(seqinfo))
 
 
-def fix_canceled_runs(seqinfo, accession2run=fix_accession2run):
+def fix_canceled_runs(seqinfo):
     """Function that adds cancelme_ to known bad runs which were forgotten
     """
     accession_number = get_unique(seqinfo, 'accession_number')
-    if accession_number in accession2run:
+    if accession_number in fix_accession2run:
         lgr.info("Considering some runs possibly marked to be "
                  "canceled for accession %s", accession_number)
-        badruns = accession2run[accession_number]
+        badruns = fix_accession2run[accession_number]
         badruns_pattern = '|'.join(badruns)
         for i, s in enumerate(seqinfo):
             if re.match(badruns_pattern, s.series_id):
@@ -391,39 +395,65 @@ def fix_canceled_runs(seqinfo, accession2run=fix_accession2run):
     return seqinfo
 
 
-def fix_dbic_protocol(seqinfo, keys=series_spec_fields, subsdict=protocols2fix):
-    """Ad-hoc fixup for existing protocols
+def fix_dbic_protocol(seqinfo):
+    """Ad-hoc fixup for existing protocols.
+
+    It will operate in 3 stages on `protocols2fix` records.
+    1. consider a record which has md5sum of study_description
+    2. apply all substitutions, where key is a regular expression which
+       successfully searches (not necessarily matches, so anchor appropriately)
+       study_description
+    3. apply "catch all" substitutions in the key containing an empty string
+
+    3. is somewhat redundant since `re.compile('.*')` could match any, but is
+    kept for simplicity of its specification.
     """
+
     study_hash = get_study_hash(seqinfo)
+    study_description = get_study_description(seqinfo)
+
+    # We will consider first study specific (based on hash)
+    if study_hash in protocols2fix:
+        _apply_substitutions(seqinfo,
+                             protocols2fix[study_hash],
+                             'study (%s) specific' % study_hash)
+    # Then go through all regexps returning regex "search" result
+    # on study_description
+    for sub, substitutions in protocols2fix.items():
+        if isinstance(sub, re_Pattern) and sub.search(study_description):
+            _apply_substitutions(seqinfo,
+                                 substitutions,
+                                 '%r regex matching' % sub.pattern)
+    # and at the end - global
+    if '' in protocols2fix:
+        _apply_substitutions(seqinfo, protocols2fix[''], 'global')
+
+    return seqinfo
 
-    if study_hash not in subsdict:
-        raise ValueError("I don't know how to fix {0}".format(study_hash))
 
-    # need to replace both protocol_name series_description
-    substitutions = subsdict[study_hash]
+def _apply_substitutions(seqinfo, substitutions, subs_scope):
+    lgr.info("Considering %s substitutions", subs_scope)
     for i, s in enumerate(seqinfo):
         fixed_kwargs = dict()
-        for key in keys:
-            value = getattr(s, key)
+        # need to replace both protocol_name series_description
+        for key in series_spec_fields:
+            oldvalue = value = getattr(s, key)
             # replace all I need to replace
             for substring, replacement in substitutions:
                 value = re.sub(substring, replacement, value)
+            if oldvalue != value:
+                lgr.info(" %s: %r -> %r", key, oldvalue, value)
             fixed_kwargs[key] = value
         # namedtuples are immutable
         seqinfo[i] = s._replace(**fixed_kwargs)
 
-    return seqinfo
-
 
 def fix_seqinfo(seqinfo):
     """Just a helper on top of both fixers
     """
     # add cancelme to known bad runs
     seqinfo = fix_canceled_runs(seqinfo)
-    study_hash = get_study_hash(seqinfo)
-    if study_hash in protocols2fix:
-        lgr.info("Fixing up protocol for {0}".format(study_hash))
-        seqinfo = fix_dbic_protocol(seqinfo)
+    seqinfo = fix_dbic_protocol(seqinfo)
     return seqinfo
 
 

heudiconv/heuristics/test_reproin.py

@@ -2,6 +2,10 @@
 # Tests for reproin.py
 #
 from collections import OrderedDict
+from mock import patch
+import re
+
+from . import reproin
 from .reproin import (
     filter_files,
     fix_canceled_runs,
@@ -78,7 +82,8 @@ def test_fix_canceled_runs():
         'accession1': ['^01-', '^03-']
     }
 
-    seqinfo_ = fix_canceled_runs(seqinfo, fake_accession2run)
+    with patch.object(reproin, 'fix_accession2run', fake_accession2run):
+        seqinfo_ = fix_canceled_runs(seqinfo)
 
     for i, s in enumerate(seqinfo_, 1):
         output = runname
@@ -106,16 +111,20 @@ def test_fix_dbic_protocol():
                        'nochangeplease',
                        'nochangeeither')
 
-
     seqinfos = [seq1, seq2]
-    keys = ['field1']
-    subsdict = {
+    protocols2fix = {
         md5sum('mystudy'):
-            [('scout_run\+', 'scout'),
+            [('scout_run\+', 'THESCOUT-runX'),
              ('run-life[0-9]', 'run+_task-life')],
+        re.compile('^my.*'):
+            [('THESCOUT-runX', 'THESCOUT')],
+        # rely on 'catch-all' to fix up above scout
+        '': [('THESCOUT', 'scout')]
     }
 
-    seqinfos_ = fix_dbic_protocol(seqinfos, keys=keys, subsdict=subsdict)
+    with patch.object(reproin, 'protocols2fix', protocols2fix), \
+            patch.object(reproin, 'series_spec_fields', ['field1']):
+        seqinfos_ = fix_dbic_protocol(seqinfos)
     assert(seqinfos[1] == seqinfos_[1])
     # field2 shouldn't have changed since I didn't pass it
     assert(seqinfos_[0] == FakeSeqInfo(accession_number,
@@ -124,8 +133,9 @@ def test_fix_dbic_protocol():
                                        seq1.field2))
 
     # change also field2 please
-    keys = ['field1', 'field2']
-    seqinfos_ = fix_dbic_protocol(seqinfos, keys=keys, subsdict=subsdict)
+    with patch.object(reproin, 'protocols2fix', protocols2fix), \
+            patch.object(reproin, 'series_spec_fields', ['field1', 'field2']):
+        seqinfos_ = fix_dbic_protocol(seqinfos)
     assert(seqinfos[1] == seqinfos_[1])
     # now everything should have changed
     assert(seqinfos_[0] == FakeSeqInfo(accession_number,