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

Add gif resize support #88

Merged
merged 13 commits into from Mar 23, 2013
Merged

Add gif resize support #88

merged 13 commits into from Mar 23, 2013

Conversation

@beartung
Copy link
Contributor

@beartung beartung commented Jan 10, 2013

add gif resize, save to blob or file

@wronglink
Copy link
Contributor

@wronglink wronglink commented Jan 10, 2013

Wouldn't it be better to use sequences api for this case?

@beartung
Copy link
Contributor Author

@beartung beartung commented Jan 10, 2013

@wronglink which api? @_@

@wronglink
Copy link
Contributor

@wronglink wronglink commented Jan 10, 2013

@beartung take a look at #39 and #66.

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Jan 10, 2013

It seems complete, without using sequences API. @beartung, can you write unit tests for it?

@@ -379,6 +392,7 @@ class MagickPixelPacket(ctypes.Structure):
libmagick.GetMagickReleaseDate.argtypes = []
libmagick.GetMagickReleaseDate.restype = ctypes.c_char_p
except AttributeError:
print traceback.print_exc()

This comment has been minimized.

@dahlia

dahlia Jan 10, 2013
Collaborator

Please remove this and above import traceback.

@beartung
Copy link
Contributor Author

@beartung beartung commented Jan 10, 2013

@dahlia done
@wronglink I think it's better to resize a gif like resizing other format image : )

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Jan 11, 2013

@beartung Travis CI says its test failed. Have you run the tests? You can do that using python setup.py test command. (See the docs.)

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Jan 11, 2013

@wronglink Sorry for delaying merging sequences API (actually I had worked on this at late of the last year, but not done yet), if we has API and unit tests for it, we can reimplement it using sequences API when that is merged.

@wronglink
Copy link
Contributor

@wronglink wronglink commented Jan 11, 2013

@dahlia sure ;-)

with img.clone() as a:
assert a.size == (350, 197)
a.resize(175, 98)
a.write(filename="175_98.gif")

This comment has been minimized.

@dahlia

dahlia Jan 11, 2013
Collaborator

You probably thought Image.save(), not write().

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Jan 11, 2013

Plus, we can make crop() to work as well in the same way, and it will be more consistent.

@beartung
Copy link
Contributor Author

@beartung beartung commented Jan 12, 2013

@dahlia test fixed, I will try crop today :)

@beartung
Copy link
Contributor Author

@beartung beartung commented Jan 14, 2013

@dahlia I added crop and rotate with tests.

@beartung
Copy link
Contributor Author

@beartung beartung commented Jan 16, 2013

so... can this pr merge?

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Jan 16, 2013

@beartung I’ll eventually merge this, but I didn’t decide where to merge (master or 0.2-maintenance).

dahlia added a commit that referenced this pull request Mar 23, 2013
@dahlia dahlia merged commit 0fa7c52 into emcconville:master Mar 23, 2013
1 check passed
1 check passed
@dahlia
default The Travis build passed
Details
dahlia added a commit that referenced this pull request Mar 23, 2013
dahlia added a commit that referenced this pull request Mar 23, 2013
@dahlia
Copy link
Collaborator

@dahlia dahlia commented Mar 23, 2013

@beartung I didn’t add credit of you to changelog (ba40270) because I couldn’t find your name. I’ll surely add credit if you want to and let me know your name. 😄

@beartung
Copy link
Contributor Author

@beartung beartung commented Mar 23, 2013

@dahlia thanks for merge it. I have my profile updated. My name is Bear Dong ^O^

dahlia added a commit that referenced this pull request Mar 23, 2013
@dahlia
Copy link
Collaborator

@dahlia dahlia commented Mar 23, 2013

@beartung Done! Thanks. 😸

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants