Skip to content

Improved icns support#565

Merged
aclark4life merged 15 commits intopython-pillow:masterfrom
al45tair:icns-support
Mar 30, 2014
Merged

Improved icns support#565
aclark4life merged 15 commits intopython-pillow:masterfrom
al45tair:icns-support

Conversation

@al45tair
Copy link
Copy Markdown
Contributor

Adds support for PNG-encoded .icns files with of up to 512x512 (plus retina, so 1024x1024 at most).

@al45tair
Copy link
Copy Markdown
Contributor Author

PIL also needs support for JPEG 2000 .icns files (yes, they’re in use; an example is EyeTV’s icon), but that will have to wait until #547 has been merged.

@wiredfool
Copy link
Copy Markdown
Member

Looks pretty good to me. I know we didn't have any tests on this before, but I'd like to get some in before merging.

@al45tair
Copy link
Copy Markdown
Contributor Author

@wiredfool As before, I’m happy to add some tests. Does Pillow have its own icon? If it does, the test cases should probably use an .icns file of that. If not, maybe it should have? Thoughts?

@wiredfool
Copy link
Copy Markdown
Member

There's no icon, and as far as I can tell no branding to turn into an icon. I'm open to improvements there. @aclark4life?

@aclark4life
Copy link
Copy Markdown
Member

Yes! Love the idea of adding icon support

@wiredfool
Copy link
Copy Markdown
Member

I was meaning an icon for Pillow. Branding. Publicity. Fame. Fortune!. or at least a beer. or something.

@aclark4life
Copy link
Copy Markdown
Member

Ah! Don't care. Sure, why not.

@aclark4life
Copy link
Copy Markdown
Member

@al45tair Can you address @wiredfool 's conditional import comment so we can merge this? Thanks

@aclark4life aclark4life added this to the 2.4.0 milestone Mar 25, 2014
@wiredfool
Copy link
Copy Markdown
Member

And tests.

@wiredfool
Copy link
Copy Markdown
Member

Not to be impatient about them or anything.

@aclark4life
Copy link
Copy Markdown
Member

Well, the 2.4.0 deadline is coming…

@al45tair
Copy link
Copy Markdown
Contributor Author

@aclark4life The conditional import comment refers to JPEG 2000 support, which (intentionally) isn't included in this particular iteration of the IcnsFilePlugin code, so I don’t think that’s a blocker for merging, unless you also want me to do the same for the use of the PNG plugin?

@wiredfool I'll knock together some simple tests today.

@al45tair
Copy link
Copy Markdown
Contributor Author

@aclark4life @wiredfool OK, tests added, and a pre-existing bug affecting Python 3 fixed. Also, I don't know what you think about the icon in Images/pillow.icns? The image files on which it’s based are both mine (I took the photos in question), so there aren’t any copyright issues.

@al45tair
Copy link
Copy Markdown
Contributor Author

@aclark4life @wiredfool Since the JPEG 2000 support was merged, I’ve merged in my JPEG 2000 .icns support and added another test image. The JPEG 2000 code is conditionally imported, as requested.

aclark4life added a commit that referenced this pull request Mar 30, 2014
@aclark4life aclark4life merged commit 6e6bc21 into python-pillow:master Mar 30, 2014
@al45tair al45tair deleted the icns-support branch April 1, 2014 15:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants