Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 6 commits into from
Mar 6, 2020

Conversation

yarikoptic
Copy link
Member

First commit in the series describes problem at large

For some data we cannot rely on reliable presence of consistent study_description
across scans.  That results in the necessity to duplicate the same protocols2fix
records for multiple (not known in advance) study descriptions (actually -- their
hashes, which makes it even more difficult).  But in the cases with custom
overloads of protocols2fix, it is desired to just provide global rewrite rules,
which could be applied to any collection since they did follow some convention.

Now, with an empty "hash", those rules would be applied last, i.e. after possible
study-specific fixups applied.

As part of the solution I removed preliminary check to skip a call to
fix_dbic_protocol altogether.  It should be better this way.

I also removed binding of series_spec_fields and protocols2fix within function
signature.  It should allow easier overloading at the module level when needed

and subsequent commits improved it even further, but altogether removing function signature level bindings, adding matching study_description using regular expression (so it could then be crafted per lab etc).

Example log output on the adjusted tests:

INFO: Considering study (0e4497bdaae2608324a85be1f54f60ed) specific substitutions
INFO:  field1: '02-anat-scout_run+_MPR_sag' -> '02-anat-THESCOUT-runX_MPR_sag'
INFO: Considering '^my.*' regex matching substitutions
INFO:  field1: '02-anat-THESCOUT-runX_MPR_sag' -> '02-anat-THESCOUT_MPR_sag'
INFO: Considering global substitutions
INFO:  field1: '02-anat-THESCOUT_MPR_sag' -> '02-anat-scout_MPR_sag'
INFO: Considering study (0e4497bdaae2608324a85be1f54f60ed) specific substitutions
INFO:  field2: '11-func_run-life2_acq-2mm692' -> '11-func_run+_task-life_acq-2mm692'
INFO: Considering '^my.*' regex matching substitutions
INFO: Considering global substitutions

…fixups for any study

For some data we cannot rely on reliable presence of consistent study_description
across scans.  That results in the necessity to duplicate the same protocols2fix
records for multiple (not known in advance) study descriptions (actually -- their
hashes, which makes it even more difficult).  But in the cases with custom
overloads of protocols2fix, it is desired to just provide global rewrite rules,
which could be applied to any collection since they did follow some convention.

Now, with an empty "hash", those rules would be applied last, i.e. after possible
study-specific fixups applied.

As part of the solution I removed preliminary check to skip a call to
fix_dbic_protocol altogether.  It should be better this way.

I also removed binding of series_spec_fields and protocols2fix within function
signature.  It should allow easier overloading at the module level when needed
…natures

Binding in signatured forbided easy rebinding of them to new values at the module level.
Example of the log now

    heudiconv/heuristics/test_reproin.py::test_fix_dbic_protocol INFO: Considering study (0e4497bdaae2608324a85be1f54f60ed) specific substitutions
    INFO:  field1: '02-anat-scout_run+_MPR_sag' -> '02-anat-THESCOUT_MPR_sag'
    INFO: Considering global substitutions
    INFO:  field1: '02-anat-THESCOUT_MPR_sag' -> '02-anat-scout_MPR_sag'
    INFO: Considering study (0e4497bdaae2608324a85be1f54f60ed) specific substitutions
    INFO:  field2: '11-func_run-life2_acq-2mm692' -> '11-func_run+_task-life_acq-2mm692'
    INFO: Considering global substitutions
@yarikoptic yarikoptic added this to the 0.6.1 milestone Mar 4, 2020
@yarikoptic
Copy link
Member Author

eh, 3.5 and 3.6 " AttributeError: module 're' has no attribute 'Pattern'"

@codecov
Copy link

codecov bot commented Mar 4, 2020

Codecov Report

Merging #425 into master will increase coverage by 0.13%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #425      +/-   ##
==========================================
+ Coverage   74.71%   74.84%   +0.13%     
==========================================
  Files          35       35              
  Lines        2760     2771      +11     
==========================================
+ Hits         2062     2074      +12     
+ Misses        698      697       -1     

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update fdea77c...febd0fd. Read the comment docs.

@yarikoptic
Copy link
Member Author

tests pass, changes shouldn't break anything, will merge in a day or two unless hear otherwise

@yarikoptic yarikoptic merged commit 4247eb6 into nipy:master Mar 6, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant