added 16bit support #33
Conversation
|
Thanks for the PR! As mentioned in #32 I'll be able to look over this and merge within a few days. |
There was a problem hiding this comment.
Thanks again for your interest in SLDTk! I've looked at the pull request and added some comments/feedback. Mostly I'm interested in conserving support for uint8 images, which is easily achievable by moving towards fraction based arguments and adapting ranges on the fly based on the treated image's dtype.
I've changed the status to "request changes" in case you'd like to make the tweaks yourself (or continue the discussion); if not I'd be happy to do so myself, just let me know. I'd of course be happy to add you as a contributor when your pull request is accepted.
Cheers,
Ariel
| @@ -89,7 +89,7 @@ def parse_input(config): | |||
| help="Number of slices to average to create the intensity " | |||
| "profile.") | |||
| ap.add_argument("-t", "--threshold", | |||
| type=_uint8, | |||
| type=int, | |||
There was a problem hiding this comment.
Type too broad. e.g. "-1337" would be a valid threshold which, assuming by 16-bit we're talking about uint16, is not desirable. If support for uint8 is to be kept (see below), I propose threshold and bias arguments are converted to fractions to remain agnostic of a given image's dtype.
| @@ -49,6 +49,6 @@ def correct_disk(img, disk_attr, bias, model): | |||
| disk = img[d_y-d_r:d_y+d_r, d_x-d_r:d_x+d_r].copy() | |||
| disk = (disk / flat) * bias | |||
|
|
|||
| img[d_y-d_r:d_y+d_r, d_x-d_r:d_x+d_r] = np.clip(disk, 0, 255).round() | |||
| img[d_y-d_r:d_y+d_r, d_x-d_r:d_x+d_r] = np.clip(disk, 0, 2**16 - 1).round() | |||
There was a problem hiding this comment.
Want to use np.iinfo(img.dtype).max in place of hardcoded values to conserve support for uint8 images.
| @@ -40,7 +40,7 @@ def detect_disk(img, threshold): | |||
| raise TypeError("Expected single channel (grayscale) image.") | |||
|
|
|||
| blur = cv2.GaussianBlur(img, (5, 5), 0) | |||
| mask = cv2.inRange(blur, threshold, 255) | |||
| mask = cv2.inRange(blur, threshold, 2**16 - 1) | |||
There was a problem hiding this comment.
Want to use np.iinfo(img.dtype).max in place of hardcoded values to conserve support for
| @@ -27,7 +27,7 @@ | |||
| "operations": ['all', 'correct', 'model'], | |||
| "threshold": 10, | |||
| "slices": 1000, | |||
| "bias": 175, | |||
| "bias": 44975, | |||
There was a problem hiding this comment.
Should be converted to a fraction as per comment for helpers.py
| @@ -41,8 +41,7 @@ | |||
| def main(): | |||
| args = parse_input(config) | |||
| paths = generate_output_paths(args) | |||
|
|
|||
| image = cv2.imread(args['image']) | |||
| image = cv2.imread(args['image'], -1) | |||
There was a problem hiding this comment.
Please use explicit constants instead of integer values for flags, i.e. cv2.IMREAD_UNCHANGED in this case.
I created a new branch adding support for 16bit images