-
-
Notifications
You must be signed in to change notification settings - Fork 318
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Adding a bulk resize image function #224
base: dev
Are you sure you want to change the base?
Conversation
Hey @amannaik247, thanks for raising the PR! This looks great. Will review in detail, just need one more thing - can you also check the |
Linking #213 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good overall, please check comments on refactoring
:param trigger_size: Minimum file size in bytes to trigger resizing. | ||
""" | ||
# Extract image files from the input directory | ||
file_extensions = ["jpg", "jpeg", "png", "tiff"] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move this as an cli arg, and add this as the default value. Also add a validation that only a subset of these 4 values is supported
@@ -119,3 +121,38 @@ def run_argparser(argparser): | |||
raise Exception(f"\nError: Unknown arguments: {unknown}") | |||
|
|||
return args | |||
|
|||
|
|||
def resize_image( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
let's move this function into image_utils.py instead
if max_width and width > max_width: | ||
new_height = int((max_width / width) * height) | ||
img = img.resize((max_width, new_height), Image.ANTIALIAS) | ||
|
||
if max_height and height > max_height: | ||
new_width = int((max_height / height) * width) | ||
img = img.resize((new_width, max_height), Image.ANTIALIAS) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I already have a util created for this in image_utils.py, can you check if we can use that?
|
||
if __name__ == "__main__": | ||
# Set up argument parser and parse arguments | ||
argparser = get_local_argparser() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
actually you should use add_common_args(...)
which internally uses the get_local_argparser().
I have a updated bulk_ops_common file to define image resize function which is used in the resize_image.py file . I have also changed the test shells to be portable since it was creating an issue on my side. Please review the code and let me know. Thank you!