neurodatascience / nipoppy

Lightweight framework for neuroimaging-clinical data organization/processing
https://nipoppy.readthedocs.io/en/latest/
MIT License
10 stars 19 forks source link

[ENH] Update global config specification #238

Closed michellewang closed 4 weeks ago

michellewang commented 1 month ago

Closes #223, closes #251

Summary:

  1. PipelineConfig changed to have an additional NAME and VERSION field, both of which are required
  2. New BidsPipelineConfig which is the same as PipelineConfig except that it has an additional required field STEP
  3. New GLOBALS field in the config, to address feedback on having to replace the same value multiple times when starting from the sample config file. Also: can basically serve as a top-level/env config things, which we discussed before
  4. The BIDS field has been renamed to BIDS_PIPELINES, to better parallel PROC_PIPELINES
  5. Update layout to use global_config.json (singular) instead of global_configs.json (plural)
  6. Update sample_global_config.json to only have a single version for each processing pipeline
codecov[bot] commented 1 month ago

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 98.33%. Comparing base (8991806) to head (de1cf71).

:exclamation: Current head de1cf71 differs from pull request most recent head fa06665

Please upload reports for the commit fa06665 to get more accurate results.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #238 +/- ## ========================================== + Coverage 97.56% 98.33% +0.77% ========================================== Files 26 28 +2 Lines 1643 1744 +101 ========================================== + Hits 1603 1715 +112 + Misses 40 29 -11 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

nikhil153 commented 1 month ago

Re: global_config substitutions, I was thinking following alternative to avoid loading, regexing, and regenerating json objects. It could be my obsolete way of thinking, but may offer a cleaner way than regex matching. The idea would be to use eval() function. We put the top level "SUBSTITUTION VARS" inside the globals and the variables in the json block holding a given variable inside locals.

sample_json

{
"SUBS" : {
        "PIPE" : "heudiconv"
    },

    "BIDS": {
        "heudiconv": {
            "VERSION": "0.11.6",    
            "CONTAINER": "f'{pipe}_{VERSION}.sif'",
            "URL": ""
        },
}

sample eval() code

# read global configs
global_config_file = "sample_global_configs.json"
with open(global_config_file, 'r') as f:
    global_configs = json.load(f)

SUBS = global_configs["SUBS"]

bids_configs = global_configs["BIDS"]
heudiconv_configs = bids_configs["heudiconv"]

CONTAINER = heudiconv_configs["CONTAINER"]

# dynamic eval
eval(CONTAINER, SUBS, heudiconv_configs)

This may require try...catch, but shouldn't be too difficult. Thoughts?