Skip to content
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

Fix resize function #110

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Conversation

vishwesh5
Copy link

Resolves #109

@vishwesh5
Copy link
Author

Sorry that was a typo.

@abhiTronix
Copy link
Contributor

abhiTronix commented Dec 21, 2018

@vishwesh5, I think you're interpreting the library's resize function wrong. This is not a bug or mistake in the library's code but rather a feature instead. The main idea here is to keep the aspect ratio of the image proportional after resizing which is certainly missing in OpenCV. You may want to check this awesome article by Adrian https://www.pyimagesearch.com/2014/01/20/basic-image-manipulations-in-python-and-opencv-resizing-scaling-rotating-and-cropping/ , may answer you properly that why this function is created in the first place.

@vishwesh5
Copy link
Author

Hi @abhiTronix ! I understand that. Even if you state it as a feature, there is a clear preference given for width rather than height. What you are referring to as a feature in itself is acting as a restriction on the resize function. If a user actually wants to distort the image, maybe for some data preprocessing step, then the imutils function fails.
Not only that, because of the current if-else condition, a new user will end up with trial and error to realize why the function is not giving the correct width or height.
Even if you want to preserve the aspect ratio, it would be better to have a simple check like this:

if width is not None and height is not None:
    print("Some note saying that height will be overridden to preserve the aspect ratio")

@abhiTronix
Copy link
Contributor

abhiTronix commented Dec 21, 2018

@vishwesh5

If a user actually wants to distort the image, maybe for some data preprocessing step, then the imutils function fails.

Ok, I get it, You're talking about the case where width and height both are to be manipulated at the same time, right?. Yes, this feature is still missing. Nice 👍

Also in your PR:
https://github.com/jrosebr1/imutils/blob/e46b86247777a1c3b448a663526d899fbfe6ecd0/imutils/convenience.py#L77 if instead of elif will also do.

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.

None yet

2 participants