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

Added Image.read() and Image.clear() #58

Merged
merged 7 commits into from Sep 18, 2012
Merged

Conversation

@pflorczyk
Copy link
Contributor

@pflorczyk pflorczyk commented Sep 17, 2012

I moved file reading related code into new method Image.read(). I also made few changes to allow calling Image without parameters. With new method Image.clear() it's now possible to reuse single Image object with multiple files:

img = Image()
img.read(filename='test1.png')
#do some operations
img.clear() # <- this clears existing wand
img.read(filename='test2.png')

It should now be possible to add functionality to close #28 without polluting Image constructor (I'm working on this).

@pflorczyk
Copy link
Contributor Author

@pflorczyk pflorczyk commented Sep 17, 2012

Could we treat blob as an edge case scenario accessible only via Image.read_blob() ? It would simplify Image.__init__() and Image.read(). We could do something like this:

def read(self, file):

and modify Image.__init__() to:

def __init__(self, image=None, file=None,
                 format=None, width=None, height=None, background=None):

Where file is file-like object or file path.
It would also be a good idea to create Image.blank(width, height, background) and remove them from constructor...

@wronglink
Copy link
Contributor

@wronglink wronglink commented Sep 18, 2012

Could we treat blob as an edge case scenario accessible only via Image.read_blob() ? It would simplify Image.init() and Image.read().

As I think, it's quite good idea to merge 2 parameters (file and filename) into 1 (like PIL do it on it's open() method). But I dont think that image creation via blob parameter is an exclusion of common use case.

BTH don't forget about image method save() if you will merge file parameters.

@dahlia
Copy link
Collaborator

@dahlia dahlia commented Sep 18, 2012

Could we treat blob as an edge case scenario accessible only via Image.read_blob() ? It would simplify Image.__init__() and Image.read().

@pflorczyk As @wronglink wrote, reading image from blob is pretty common, so IMO we’d better keep parameters explicit.

Anyway this patch seems great. I’ll merge this.

dahlia added a commit that referenced this pull request Sep 18, 2012
Added Image.read() and Image.clear()
@dahlia dahlia merged commit 33fd01a into emcconville:master Sep 18, 2012
1 check passed
1 check passed
default The Travis build passed
Details
dahlia added a commit that referenced this pull request Sep 18, 2012
dahlia added a commit that referenced this pull request Sep 18, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.