Skip to content

Commit f5c45a2

Browse files
authored
RF: Polish wrapper logic (#183)
* RF: Polish wrapper logic - Displaying help should return 0 exit code - Remove dual `--help`/`--version` behavior - only show wrapper options. * MAINT: Update copyright year
1 parent 1e3f785 commit f5c45a2

File tree

1 file changed

+33
-61
lines changed

1 file changed

+33
-61
lines changed

wrapper/nibabies_wrapper.py

Lines changed: 33 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -20,9 +20,7 @@
2020
import subprocess
2121

2222
__version__ = '99.99.99'
23-
__copyright__ = (
24-
'Copyright 2021, The NiPreps Developers'
25-
)
23+
__copyright__ = 'Copyright 2022, The NiPreps Developers'
2624
__bugreports__ = 'https://github.com/nipreps/nibabies/issues'
2725

2826
MISSING = """
@@ -120,9 +118,7 @@ def add_mount(self, src, dst, read_only=True):
120118
dst : absolute container path
121119
read_only : disable writing to bound path
122120
"""
123-
self.mounts.append(
124-
"{0}:{1}{2}".format(src, dst, ":ro" if read_only else "")
125-
)
121+
self.mounts.append("{0}:{1}{2}".format(src, dst, ":ro" if read_only else ""))
126122

127123
def check_install(self):
128124
"""Verify that the service is installed and the user has permission to
@@ -153,9 +149,7 @@ def check_install(self):
153149
def check_image(self, image):
154150
"""Check whether image is present on local system"""
155151
if self.service == "docker":
156-
ret = subprocess.run(
157-
["docker", "images", "-q", image], stdout=subprocess.PIPE
158-
)
152+
ret = subprocess.run(["docker", "images", "-q", image], stdout=subprocess.PIPE)
159153
return bool(ret.stdout)
160154
elif self.service == "singularity":
161155
# check if the image file exists
@@ -349,9 +343,7 @@ def _is_file(path, parser):
349343
"""Ensure a given path exists and it is a file."""
350344
path = os.path.abspath(path)
351345
if not os.path.isfile(path):
352-
raise parser.error(
353-
"Path should point to a file (or symlink of file): <%s>." % path
354-
)
346+
raise parser.error("Path should point to a file (or symlink of file): <%s>." % path)
355347
return path
356348

357349
parser = argparse.ArgumentParser(
@@ -467,14 +459,14 @@ def _is_file(path, parser):
467459
"--segmentation-atlases-dir",
468460
metavar="PATH",
469461
type=os.path.abspath,
470-
help="Directory containing prelabeled segmentations to use for JointLabelFusion."
462+
help="Directory containing prelabeled segmentations to use for JointLabelFusion.",
471463
)
472464
g_wrap.add_argument(
473465
"--derivatives",
474466
nargs="+",
475467
metavar="PATH",
476468
type=os.path.abspath,
477-
help="One or more directory containing pre-computed derivatives"
469+
help="One or more directory containing pre-computed derivatives",
478470
)
479471
g_wrap.add_argument(
480472
"--bids-filter-file",
@@ -533,9 +525,7 @@ def _is_file(path, parser):
533525
help="Run container with a different network driver "
534526
'("none" to simulate no internet connection)',
535527
)
536-
g_dev.add_argument(
537-
"--no-tty", action="store_true", help="Run docker without TTY flag -it"
538-
)
528+
g_dev.add_argument("--no-tty", action="store_true", help="Run docker without TTY flag -it")
539529

540530
return parser
541531

@@ -552,44 +542,35 @@ def main():
552542
return
553543

554544
# Set help if no directories set
555-
if not all((opts.service, opts.bids_dir, opts.output_dir)):
545+
if opts.help or not all((opts.service, opts.bids_dir, opts.output_dir)):
556546
parser.print_help()
557-
return 1
547+
return
558548

559549
container = ContainerManager(opts.service, image=opts.image)
560550
check = container.check_install()
561551
if check < 1:
562-
if opts.version:
563-
print("nibabies wrapper {!s}".format(__version__))
564-
if opts.help:
565-
parser.print_help()
566552
if check == -1:
567553
print(
568-
"nibabies: Could not find %s command... Is it installed?"
569-
% opts.service
554+
"nibabies: Could not find %s command... Is it installed?" % opts.service,
555+
file=sys.stderr,
570556
)
571557
else:
572558
print(
573-
"nibabies: Make sure you have permission to run '%s'" % opts.service
559+
"nibabies: Make sure you have permission to run '%s'" % opts.service,
560+
file=sys.stderr,
574561
)
575562
return 1
576563

577-
# For --help or --version, ask before downloading an image
578564
if not container.check_image(opts.image):
579565
resp = "Y"
580-
if opts.version:
581-
print("nibabies wrapper {!s}".format(__version__))
582-
if opts.help:
583-
parser.print_help()
584-
if opts.version == "singularity":
585-
print("Singularity images must already exist locally.")
566+
if opts.service == "singularity":
567+
print("Singularity image must already exist locally.", file=sys.stderr)
568+
return 1
569+
try:
570+
resp = input(MISSING.format(opts.image))
571+
except KeyboardInterrupt:
572+
print()
586573
return 1
587-
if opts.version or opts.help:
588-
try:
589-
resp = input(MISSING.format(opts.image))
590-
except KeyboardInterrupt:
591-
print()
592-
return 1
593574
if resp not in ("y", "Y", ""):
594575
return 0
595576
print("Downloading. This may take a while...")
@@ -602,10 +583,7 @@ def main():
602583
"Do you have permission to run docker?"
603584
)
604585
return 1
605-
if (
606-
not (opts.help or opts.version or "--reports-only" in unknown_args)
607-
and mem_total < 8000
608-
):
586+
if "--reports-only" not in unknown_args and mem_total < 8000:
609587
print(
610588
"Warning: <8GB of RAM is available within your environment.\n"
611589
"Some parts of nibabies may fail to complete."
@@ -624,11 +602,7 @@ def main():
624602

625603
if opts.service == "docker":
626604
if not opts.no_tty:
627-
if opts.help:
628-
# TTY can corrupt stdout
629-
container.add_cmd("-i")
630-
else:
631-
container.add_cmd("-it")
605+
container.add_cmd("-it")
632606
if opts.user:
633607
container.add_cmd(("-u", opts.user))
634608
if opts.network:
@@ -718,9 +692,7 @@ def main():
718692
if space.split(":")[0] not in (TF_TEMPLATES + NONSTANDARD_REFERENCES):
719693
tpl = os.path.basename(space)
720694
if not tpl.startswith("tpl-"):
721-
raise RuntimeError(
722-
"Custom template %s requires a `tpl-` prefix" % tpl
723-
)
695+
raise RuntimeError("Custom template %s requires a `tpl-` prefix" % tpl)
724696
target = "/home/fmriprep/.cache/templateflow/" + tpl
725697
container.add_mount(os.path.abspath(space), target)
726698
spaces.append(tpl[4:])
@@ -742,16 +714,16 @@ def main():
742714

743715
# Override help and version to describe underlying program
744716
# Respects '-i' flag, so will retrieve information from any image
745-
if opts.help:
746-
container.add_cmd("-h")
747-
targethelp = subprocess.check_output(container.command).decode()
748-
print(merge_help(parser.format_help(), targethelp))
749-
return 0
750-
elif opts.version:
751-
# Get version to be run and exit
752-
container.add_cmd("--version")
753-
ret = subprocess.run(container.command)
754-
return ret.returncode
717+
# if opts.help:
718+
# container.add_cmd("-h")
719+
# targethelp = subprocess.check_output(container.command).decode()
720+
# print(merge_help(parser.format_help(), targethelp))
721+
# return 0
722+
# elif opts.version:
723+
# # Get version to be run and exit
724+
# container.add_cmd("--version")
725+
# ret = subprocess.run(container.command)
726+
# return ret.returncode
755727

756728
if not opts.shell:
757729
container.add_cmd(main_args)

0 commit comments

Comments
 (0)