Skip to content

Add image loading and validation features with unit tests#25

Merged
not-lain merged 10 commits intonot-lain:mainfrom
BouajilaHamza:add-load_multiple-imgs-feature
Mar 18, 2025
Merged

Add image loading and validation features with unit tests#25
not-lain merged 10 commits intonot-lain:mainfrom
BouajilaHamza:add-load_multiple-imgs-feature

Conversation

@BouajilaHamza
Copy link
Contributor

@BouajilaHamza BouajilaHamza commented Mar 12, 2025

  1. Introduce a new function for loading multiple images with support for various input types and parallel processing.
  2. Enhance the existing image loading function with validation and error handling.
  3. Include unit tests to ensure the functionality of the new features.

Copy link
Owner

@not-lain not-lain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome work @BouajilaHamza , thanks a lot for your contribution, I left some comments and suggestions, could you attend to them

else:
raise ValueError(f"Unsupported output type: {output_type}")
# List of sources
image_paths = imgs
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we could have looped over imgs directly instead of creating an extra parameter, mean in this case we case image_paths could be a list of pillow images so it's not technically paths per say

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm i got that.
Insightful !

Comment on lines +288 to +307
def validate_image(img: Image.Image) -> tuple[bool, str]:
"""Validates an image for basic requirements.

Args:
img: PIL Image to validate

Returns:
tuple[bool, str]: (is_valid, error_message)
"""
if not isinstance(img, Image.Image):
return False, "Input is not a PIL Image"

if img.size[0] * img.size[1] == 0:
return False, "Image has zero dimensions"

try:
img.verify()
return True, ""
except Exception as e:
return False, f"Image verification failed: {str(e)}"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you add a test for this or an example, I haven't really encountered similar errors with loadimg before

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Okey i will check that

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@not-lain I have added simple testing function in loadimg file, you can check it

BouajilaHamza and others added 4 commits March 14, 2025 22:03
Co-authored-by: Hafedh Hichri <70411813+not-lain@users.noreply.github.com>
Co-authored-by: Hafedh Hichri <70411813+not-lain@users.noreply.github.com>
Copy link
Owner

@not-lain not-lain left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks a lot @BouajilaHamza

Comment on lines +163 to +164
# Create an image with zero width to simulate zero dimensions
img = Image.new("RGB", (0, 10))
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see why we need the verification now, thanks for iterating

@not-lain not-lain changed the base branch from dev to main March 17, 2025 18:51
@not-lain
Copy link
Owner

LGTM, merging
Thanks for adding this !

@not-lain not-lain merged commit 6f5c8e9 into not-lain:main Mar 18, 2025
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants