ENH: Add nib-roi command to crop (maybe flip) axes #947

effigies · 2020-08-20T20:32:36Z

Quick command to easily crop images using slice notation. Inspired by fslroi.

Features:

Python slicing syntax.
Scale factors and original values preserved exactly.
Flips on spatial axes are permitted with a -1 step (but downsampling with other steps is not...)

Safety checks ~~that are not there:~~

Zero-length slices are ~~not~~ checked for, e.g. -i 1:1 or -j 5:6:-1.
Using the same input and output file on an uncompressed image ~~will cause a BusError as the image being read will be truncated.~~ will avoid mmaping and is tested for.

With a basic usable tool, I'm not inclined to push further today, so I figured I'd give people a chance to comment.

pep8speaks · 2020-08-20T20:32:40Z

Hello @effigies, Thank you for updating!

Cheers! There are no style issues detected in this Pull Request. 🍻 To test for issues locally, pip install flake8 and then run flake8 nibabel.

Comment last updated at 2020-10-09 12:41:12 UTC

codecov · 2020-08-20T20:38:51Z

Codecov Report

Merging #947 into master will increase coverage by 0.04%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##           master     #947      +/-   ##
==========================================
+ Coverage   91.87%   91.91%   +0.04%     
==========================================
  Files          98       99       +1     
  Lines       12451    12514      +63     
  Branches     2193     2204      +11     
==========================================
+ Hits        11439    11502      +63     
  Misses        678      678              
  Partials      334      334

Impacted Files	Coverage Δ
nibabel/cmdline/roi.py	`100.00% <100.00%> (ø)`

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 2f918b1...4623e12. Read the comment docs.

effigies · 2020-09-18T12:54:58Z

This could use a review. @chrispycheng any interest?

chrispycheng · 2020-09-19T08:25:20Z

@effigies Sure, let me get back to you! Plate's a bit stacked ATM

effigies · 2020-09-19T11:24:25Z

No worries.

chrispycheng

Ran the function and looks awesome! Skimmed the code and raised just one point of concern for your review. Also, figured I wouldn't rehash codecov, esp. since areas lacking coverage seem pretty insignificant.

nibabel/cmdline/tests/test_roi.py

effigies · 2020-09-23T14:50:08Z

Thanks for the review. I went ahead and added tests that should cover the remaining lines.

effigies · 2020-10-09T13:16:44Z

The quest for coverage ended up finding an edge case. Full coverage and tests passing.

@chrispycheng Do you have time for a final review?

chrispycheng · 2020-10-15T02:29:00Z

I’m smothered with work right now.. maybe in 2 weeks? is that OK?

…

Sent from my iPhone On Oct 9, 2020, at 21:17, Chris Markiewicz <[email protected]> wrote: The quest for coverage ended up finding an edge case. Full coverage and tests passing. @chrispycheng<https://github.com/chrispycheng> Do you have time for a final review? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#947 (comment)>, or unsubscribe<https://github.com/notifications/unsubscribe-auth/AGU42AIZXEESWAMJ73AM4FDSJ4EM5ANCNFSM4QGSIMKQ>.

effigies · 2020-10-15T16:47:20Z

No worries. Thanks for the heads up.

mgxd

Looks good - few comments

nibabel/cmdline/tests/test_roi.py

nibabel/cmdline/roi.py

mgxd · 2020-10-15T19:34:34Z

nibabel/cmdline/roi.py

+        print("Could not slice image. Full traceback follows.")
+        raise
+    nb.save(sliced_img, opts.out_file)
+    return 0


is this needed?

Suggested change

return 0

Not really, makes the output of calling main() identical to the exit code, though. If I change this, I think I'll need to update a test, but I can.

nbd 🤷‍♂️

mgxd · 2020-10-15T19:45:49Z

nibabel/cmdline/roi.py

+    parser.add_argument("-t", metavar="T1:T2", help="Start/stop along fourth axis (0-indexed)")
+    parser.add_argument("in_file", help="Image file to crop")
+    parser.add_argument("out_file", help="Output file name")
+


should this show the help if no arguments are provided?

Suggested change

if not args:

parser.print_help()

sys.exit(1)

You do at least get usage:

$ nib-roi usage: nib-roi [-h] [--version] [-i I1:I2[:-1]] [-j J1:J2[:-1]] [-k K1:K2[:-1]] [-t T1:T2] in_file out_file nib-roi: error: the following arguments are required: in_file, out_file $ nib-roi --help usage: nib-roi [-h] [--version] [-i I1:I2[:-1]] [-j J1:J2[:-1]] [-k K1:K2[:-1]] [-t T1:T2] in_file out_file Crop images to a region of interest positional arguments: in_file Image file to crop out_file Output file name optional arguments: -h, --help show this help message and exit --version show program's version number and exit -i I1:I2[:-1] Start/stop [flip] along first axis (0-indexed) -j J1:J2[:-1] Start/stop [flip] along second axis (0-indexed) -k K1:K2[:-1] Start/stop [flip] along third axis (0-indexed) -t T1:T2 Start/stop along fourth axis (0-indexed) If a start or stop value is omitted, the start or end of the axis is assumed.

We don't have a terribly consistent style, to go by:

$ nib-ls; echo $? 0 $ nib-conform; echo $? usage: nib-conform [-h] [--out-shape OUT_SHAPE OUT_SHAPE OUT_SHAPE] [--voxel-size VOXEL_SIZE VOXEL_SIZE VOXEL_SIZE] [--orientation ORIENTATION] [-f] [-V] infile outfile nib-conform: error: the following arguments are required: infile, outfile 2 $ nib-diff; echo $? Traceback (most recent call last): File "/home/cjmarkie/anaconda3/envs/nibabel_dev/bin/nib-diff", line 11, in <module> load_entry_point('nibabel', 'console_scripts', 'nib-diff')() File "/home/cjmarkie/Projects/nipy/nibabel/nibabel/cmdline/diff.py", line 362, in main dtype=opts.dtype File "/home/cjmarkie/Projects/nipy/nibabel/nibabel/cmdline/diff.py", line 315, in diff assert len(files) >= 2, "Please enter at least two files" AssertionError: Please enter at least two files 1 $ nib-tck2trk; echo $? usage: nib-tck2trk [-h] [-f] anatomy tractogram [tractogram ...] nib-tck2trk: error: the following arguments are required: anatomy, tractogram 2

But on the whole I think I'm okay with argparse defaults.

effigies · 2020-10-16T13:32:39Z

Since none of the comments seemed blocking, I'm going to go ahead and merge. Thanks for the reviews, @chrispycheng, @mgxd!

effigies added 2 commits September 15, 2020 09:09

ENH: Add nib-roi command to crop (maybe flip) axes

ace3e4e

ENH: Cover edge cases like non-scaling images, bad slices

ab96044

effigies force-pushed the enh/nib-roi branch from 80d6761 to 7e6bbf7 Compare September 15, 2020 13:09

effigies added this to the 3.2.0 milestone Sep 15, 2020

TEST: Test nib-roi and helpers

310a242

effigies force-pushed the enh/nib-roi branch from 7e6bbf7 to 310a242 Compare September 15, 2020 13:44

chrispycheng reviewed Sep 23, 2020

View reviewed changes

nibabel/cmdline/tests/test_roi.py Outdated Show resolved Hide resolved

effigies added 2 commits September 23, 2020 09:11

Update nibabel/cmdline/tests/test_roi.py

e969374

TEST: Check MINC-2, Analyze, and main()

d0a6425

effigies added 2 commits October 9, 2020 08:39

TEST: Use MGHImage to avoid scipy dependency

4e985a7

FIX: Improved test for scaling

4623e12

mgxd reviewed Oct 15, 2020

View reviewed changes

effigies merged commit 917afab into nipy:master Oct 16, 2020

effigies deleted the enh/nib-roi branch October 16, 2020 13:32

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

ENH: Add nib-roi command to crop (maybe flip) axes #947

ENH: Add nib-roi command to crop (maybe flip) axes #947

effigies commented Aug 20, 2020 •

edited

Loading

pep8speaks commented Aug 20, 2020 •

edited

Loading

codecov bot commented Aug 20, 2020 •

edited

Loading

effigies commented Sep 18, 2020

chrispycheng commented Sep 19, 2020

effigies commented Sep 19, 2020

chrispycheng left a comment

effigies commented Sep 23, 2020

effigies commented Oct 9, 2020

chrispycheng commented Oct 15, 2020 via email

effigies commented Oct 15, 2020

mgxd left a comment

mgxd Oct 15, 2020

effigies Oct 15, 2020

mgxd Oct 15, 2020

mgxd Oct 15, 2020

effigies Oct 15, 2020

effigies commented Oct 16, 2020

+    if not args:
+        parser.print_help()
+        sys.exit(1)

ENH: Add nib-roi command to crop (maybe flip) axes #947

ENH: Add nib-roi command to crop (maybe flip) axes #947

Conversation

effigies commented Aug 20, 2020 • edited Loading

pep8speaks commented Aug 20, 2020 • edited Loading

Comment last updated at 2020-10-09 12:41:12 UTC

codecov bot commented Aug 20, 2020 • edited Loading

Codecov Report

effigies commented Sep 18, 2020

chrispycheng commented Sep 19, 2020

effigies commented Sep 19, 2020

chrispycheng left a comment

Choose a reason for hiding this comment

effigies commented Sep 23, 2020

effigies commented Oct 9, 2020

chrispycheng commented Oct 15, 2020 via email

effigies commented Oct 15, 2020

mgxd left a comment

Choose a reason for hiding this comment

mgxd Oct 15, 2020

Choose a reason for hiding this comment

effigies Oct 15, 2020

Choose a reason for hiding this comment

mgxd Oct 15, 2020

Choose a reason for hiding this comment

mgxd Oct 15, 2020

Choose a reason for hiding this comment

effigies Oct 15, 2020

Choose a reason for hiding this comment

effigies commented Oct 16, 2020

effigies commented Aug 20, 2020 •

edited

Loading

pep8speaks commented Aug 20, 2020 •

edited

Loading

codecov bot commented Aug 20, 2020 •

edited

Loading