Image resize and preserve aspect ratio : #609 #3301
Conversation
… resize_and_maintain_aspect_ratio
There was a problem hiding this comment.
@NeerajKomuravalli - Thanks for the pull request.
We'll need to add some unit test for this new functionality. I think test_image_type.py is probably the right place for those unit tests.
| raise ValueError("annotations are expected in the form of list") | ||
|
|
||
| for ann_index in range(len(annotations)): | ||
| annotations[ann_index]["coordinates"]["height"] = annotations[ann_index]["coordinates"]["height"]*target_image_shape[1]/original_image_shape[1] |
There was a problem hiding this comment.
Recalculating target_image_shape[0]/original_image_shape[0] and target_image_shape[1]/original_image_shape[1] twice for each element of annotations is not optimal.
It's better to calculate those values once, outside of the loop, and then just use them inside the loop.
There was a problem hiding this comment.
I agree, I will fix it.
| target_image_shape : tuple | list | ||
| Target image dimesions in the format of (width, height) | ||
| """ | ||
| if type(annotations) is not list: |
There was a problem hiding this comment.
This should work for SArray too. I think with out this check it would work for SArrays.
There was a problem hiding this comment.
I actually did not intent it work for SArray but I think make it work for SArray makes a lot of sense. Will modify it to work for SArray as well.
| """ | ||
|
|
||
| from ...data_structures.sarray import SArray as _SArray | ||
| import turicreate as _tc |
There was a problem hiding this comment.
You don't need this line. It's already imported at the top of the file.
There was a problem hiding this comment.
Will remove it from there, also I want to point out that the same thing is present in the resize function.
| side_length_type = "min_length" | ||
| side_length = min_side_length | ||
| else: | ||
| raise ValueError("Value of min_side_length parameter cannot be negetive") |
There was a problem hiding this comment.
negetive -> negative.
There was a problem hiding this comment.
Will make the modification.
| side_length_type = "max_length" | ||
| side_length = max_side_length | ||
| else: | ||
| raise ValueError("Value of max_side_length parameter cannot be negetive") |
There was a problem hiding this comment.
negetive -> negative.
There was a problem hiding this comment.
Will make the modification.
| Resize a single image | ||
|
|
||
| >>> img = turicreate.Image('https://static.turi.com/datasets/images/sample.jpg') | ||
| >>> resized_img = turicreate.image_analysis.resize_with_original_aspect_ratio(img,min_side_length=500,max_side_length=None, channels=1) |
There was a problem hiding this comment.
With example code, we want a space after each comma.
There was a problem hiding this comment.
Will make the modification.
|
|
||
| Returns | ||
| ------- | ||
| out : turicreate.Image |
There was a problem hiding this comment.
It's could also be an SArray, if the input is an SArray, right?
There was a problem hiding this comment.
Yes, you are right. I completely missed it. Will add SArray here
| if min_side_length is None and max_side_length is None: | ||
| raise ValueError("Cannot resize when neither `min_side_length` or `max_side_length` is not provided") | ||
|
|
||
| if (min_side_length is not None and min_side_length > 0) and (max_side_length is not None and max_side_length > 0): |
There was a problem hiding this comment.
Later on you seem to be checking the case were the side lengths are negative. The error message here doesn't mention negative. You probably want to remove the negative checks here.
There was a problem hiding this comment.
oh, yeah! No need to do negative checks here.
| side_length = max_side_length | ||
| else: | ||
| raise ValueError("Value of max_side_length parameter cannot be negetive") | ||
| else: |
There was a problem hiding this comment.
I don't think you need this. I think you're already checking the case were either is provided.
There was a problem hiding this comment.
True, will make the modifications.
| ) | ||
|
|
||
|
|
||
| def _get_new_image_dimensions_with_original_aspect_ratio(image, side_length_type, side_length): |
There was a problem hiding this comment.
It would be cleaner to make this an inner function to resize_with_original_aspect_ratio.
|
linking to open issue: #609. |
|
Will add test cases In |
@TobyRoseman
As discussed,
I have added two basic functions:
resize_with_original_aspect_ratio: to resize images by preserving aspect ratio using the user given parameter =>min_side_length/max_side_length.resize_annotations: to resize annotations based on any new image dimension provided by the user.I couldn't find any test cases written for any functions in
image_analysis.pyin thetestfolder so I did not add any test cases for these as well.Let me know if any changes are required to the code submitted.