-
Notifications
You must be signed in to change notification settings - Fork 26
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
[Alpha pipeline] Add image border cropping component #147
[Alpha pipeline] Add image border cropping component #147
Conversation
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.
Thanks @ChristiaensBert!
Could you add a small README as well? Including the images from your PR description.
We should do this for every reusable component :)
examples/pipelines/simple_pipeline/components/image_cropping/fondant_component.yaml
Outdated
Show resolved
Hide resolved
def extract_width(image_bytes: bytes) -> np.in16: | ||
"""Extract the width of an image | ||
|
||
Args: | ||
image_bytes (bytes): input image in bytes | ||
|
||
Returns: | ||
np.int16: width of the image | ||
""" | ||
return np.int16( | ||
Image.open(io.BytesIO(image_bytes)).size[0] | ||
) | ||
|
||
|
||
def extract_height(image_bytes: bytes) -> np.int16: | ||
"""Extract the height of an image | ||
|
||
Args: | ||
image_bytes (bytes): input image in bytes | ||
|
||
Returns: | ||
np.int16: height of the image | ||
""" | ||
return np.int16( | ||
Image.open(io.BytesIO(image_bytes)).size[1] | ||
) |
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.
Can we combine these so we don't need to open the image twice?
We could also expect these as part of the input dataset, but I guess that would make this component less reusable, since then it would only work on datasets where this information is already available.
|
||
args: | ||
scale: | ||
description: Scale parameter used for detecting borders |
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.
These arguments are not completely clear to me. Can we explain them in a bit more detail? Either here or in the readme.
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 removed the first argument since it's not that important and added some information about the offset
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.
Thanks @ChristiaensBert :)
Left a few small comments but should be good to ship
examples/pipelines/simple_pipeline/components/image_cropping/fondant_component.yaml
Outdated
Show resolved
Hide resolved
|
||
# serialize image to JPEG | ||
crop_bytes = io.BytesIO() | ||
image_crop.save(crop_bytes, format="JPEG") |
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.
probably best to omit the format to keep it as the default one. Perhaps user start with a png image and want to keep it that way
[ | ||
"images_data", | ||
] | ||
].apply(extract_dimensions, axis=1, result_type="expand", meta={0: int, 1: int}) |
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.
can we define the meta return types explicitly and not by index?
meta={"images_width": int, "images_height": int}
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 think I tried it but it didn't work. But by having the "images_width" and height in the dataframe it gives the same results. It's weird
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.
Thanks Bert!
If you resolve the merge conflict, you can merge.
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.
Thanks Bert!
If you resolve the merge conflict, you can merge.
This PR contains the Image Cropping component. The component looks for the most common color in the border. It uses this color to calculate how much of the image border can be cropped out. If the crop is not square, it will paste a border on the shortest side to make it square again. ![d4e35776-3ce1-4157-ac1f-5b2f18ff2ad4](https://github.com/ml6team/fondant/assets/92580873/314ec0d3-3ab6-418e-8051-d9f464496b0e) ![82eeae2d-c63c-42cb-881c-3707971d043c](https://github.com/ml6team/fondant/assets/92580873/6754b418-7922-4744-8ef3-59978b07ee9d) --------- Co-authored-by: Philippe Moussalli <philippe.moussalli95@gmail.com>
This PR contains the Image Cropping component. The component looks for the most common color in the border. It uses this color to calculate how much of the image border can be cropped out. If the crop is not square, it will paste a border on the shortest side to make it square again. ![d4e35776-3ce1-4157-ac1f-5b2f18ff2ad4](https://github.com/ml6team/fondant/assets/92580873/314ec0d3-3ab6-418e-8051-d9f464496b0e) ![82eeae2d-c63c-42cb-881c-3707971d043c](https://github.com/ml6team/fondant/assets/92580873/6754b418-7922-4744-8ef3-59978b07ee9d) --------- Co-authored-by: Philippe Moussalli <philippe.moussalli95@gmail.com>
This PR contains the Image Cropping component.
The component looks for the most common color in the border. It uses this color to calculate how much of the image border can be cropped out. If the crop is not square, it will paste a border on the shortest side to make it square again.