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

Patch to support resample command #244

Merged

Conversation

@ziotibia81
Copy link
Contributor

@ziotibia81 ziotibia81 commented Jun 12, 2015

Support for ImageMagick Resample command.

http://www.imagemagick.org/Usage/resize/#resample

@ziotibia81
Copy link
Contributor Author

@ziotibia81 ziotibia81 commented Jun 12, 2015

this implement image.resample

Resample command is explaned here: http://www.imagemagick.org/Usage/resize/#resample

Corrected indentation of "library.MagickResampleImage.argtypes"
library.MagickSetIteratorIndex(self.wand, i)
library.MagickResampleImage(self.wand, x_res, y_res,
filter, blur)
#You really need to do it?

This comment has been minimized.

@emcconville

emcconville Jun 12, 2015
Owner

No, you don't really need to re-set resolution.

@emcconville
Copy link
Owner

@emcconville emcconville commented Jun 12, 2015

Awesome work! Missing unit + regression tests

def resample(self, x_res=None, y_res=None, filter='undefined', blur=1):
"""adjust the number of pixels in an image so that when displayed at the
given Resolution or Density the image will still look the same size in
real world terms.

This comment has been minimized.

@dahlia

dahlia Jun 13, 2015
Collaborator

It would be better if it follows Wand’s docstring convention.

This comment has been minimized.

@ziotibia81

ziotibia81 Jun 13, 2015
Author Contributor

Could you explain better the mistake?

This comment has been minimized.

@dahlia

dahlia Jun 20, 2015
Collaborator

Sorry for late response. Although it’s trivial, we haven’t aligned paragraphs’ left edges to leading text of content e.g.:

"""blah
   blah

but leading quote character e.g.:

"""Blah
blah

Also the leading character of every sentence should be uppercase.

((48,None), (175, 197)),
])
def test_resample_gif(density, expected_size, fx_asset):
"""Resample (Adjust nuber of pixels at the given density) the image."""

This comment has been minimized.

@ziotibia81

ziotibia81 Jun 15, 2015
Author Contributor

This test is fomally correct, but it fails.
GIF don't support "density" as standard. Maybe that setter of Image.resolution should be changed to set resolution for each frame in case of animated gif.

@emcconville emcconville added this to the 0.4.5 milestone Nov 2, 2018
@emcconville emcconville merged commit 13822a4 into emcconville:master Nov 9, 2018
1 check failed
1 check failed
continuous-integration/travis-ci/pr The Travis CI build failed
Details
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