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

add gif resize, save to blob or file

@wronglink
Copy link
Contributor

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

@beartung
Copy link
Contributor Author

@wronglink which api? @_@

@wronglink
Copy link
Contributor

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

@dahlia
Copy link
Collaborator

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()
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please remove this and above import traceback.

@beartung
Copy link
Contributor Author

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

@dahlia
Copy link
Collaborator

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 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

@dahlia sure ;-)

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

Choose a reason for hiding this comment

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

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

@dahlia
Copy link
Collaborator

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

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

@beartung
Copy link
Contributor Author

@dahlia I added crop and rotate with tests.

@beartung
Copy link
Contributor Author

so... can this pr merge?

@dahlia
Copy link
Collaborator

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
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 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

@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 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
Development

Successfully merging this pull request may close these issues.

None yet

3 participants