[ENH] Add --tar option to nipoppy run
#430
Merged
+323
−31
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
michellewang
commented
Nov 19, 2024
Comment on lines
-88
to
-94
| # load tracker configs from file | ||
| fpath_tracker_config = self.pipeline_step_config.TRACKER_CONFIG_FILE | ||
| if fpath_tracker_config is None: | ||
| raise ValueError( | ||
| f"No tracker config file specified for pipeline {self.pipeline_name}" | ||
| f" {self.pipeline_version}" | ||
| ) | ||
| # replace template strings |
There was a problem hiding this comment.
Moved to BasePipelineWorkflow
Codecov ReportAll modified and coverable lines are covered by tests ✅
🚀 New features to boost your workflow:
|
michellewang
commented
Nov 19, 2024
| @@ -113,7 +117,7 @@ def test_run_failed_cleanup(tmp_path: Path, n_success, config: Config): | |||
| ) | |||
| runner.n_success = n_success | |||
| runner.n_total = 2 | |||
| config.save(runner.layout.fpath_config) | |||
| runner.config = config | |||
mathdugre
requested changes
Nov 28, 2024
| if self.tracker_config.PARTICIPANT_SESSION_DIR is None: | ||
| raise RuntimeError( | ||
| "Tarring requested but no participant-session directory specified. " | ||
| "The PARTICIPANT_SESSION_DIR field in the tracker config must set " |
There was a problem hiding this comment.
Overall, I find the PARTICIPANT_SESSION_DIR confusing. It's not explicit that it's for the TAR dir.
There was a problem hiding this comment.
Ok, I named it like this because I thought this could be multi-purpose for derivatives discovery: if we add this path to the imaging bagel file then Neurobagel would know where to find the derivatives data (when building cohorts for example). But will ask around and see if there is a better name we could use
mathdugre
approved these changes
Dec 19, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Changes proposed in this pull request:
PARTICIPANT_SESSION_DIRPATHSor existence of a tarball atPARTICIPANT_SESSION_DIR+.tar. We could also include this path in the imaging bagel file, which would be useful for Neurobagel (but that is for a future PR)--taris set and the processing pipeline run is successful,PARTICIPANT_SESSION_DIRwill be tarred into an archive of the same name (+.tarextension) and the original directory will be deletedChecklist (for reviewers)
This section is for the PR reviewer
[BUG],[DOC],[ENH],[MAINT])Refer to NumPy Development Guide for a full list
Closes #XXXXFor new features:
For bug fixes:
There is at least one test that would fail under the original bug conditions