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

I wirte some simple demos, please merge it #40

Closed
wants to merge 105 commits into from
Closed

I wirte some simple demos, please merge it #40

wants to merge 105 commits into from

Conversation

ghost
Copy link

@ghost ghost commented Jan 15, 2013

I wirte some simple demos, please merge it.

@d-schmidt
Copy link
Contributor

The first thing I've tested is the histogram.
It didn't work
im.draft() is just pass, nothing happens there. I had to replaced it with im.convert() to get it working.
xrange() has been removed from Python 3

@aclark4life
Copy link
Member

Doh! I would love to get these in somehow at somepoint…

@ghost
Copy link
Author

ghost commented Jan 16, 2013

I do some small changes and all demos pass test on Ubuntu 12.04 64bit,
please merge it.

I have never see people use Python 3 in production,
so we should not care deprecated xrange.

@inklesspen
Copy link

I think demo code should have more comments explaining what things are doing, and the README.md should explain what each program is demonstrating.

Also, I think "pixsel" is a typo for "pixel".

@aclark4life
Copy link
Member

@shuge The master branch now supports Python 3…

@ghost
Copy link
Author

ghost commented Jan 16, 2013

Thanks.

File name and fodler name have self-explain what each program is demonstrating.

I have used range instead of xrange.

@aclark4life
Copy link
Member

Thanks, can you fix "This pull request cannot be automatically merged." ? I don't know if there are other objections but I'd prefer to merge through the web if possible.

@inklesspen
Copy link

shuge, I appreciate your taking the time to write these demos, and I want to emphasize that this is only my point of view; I can't speak for the Pillow maintainers here.

But your English is not very good and I don't think the filenames are self-explanatory enough; tutorial code needs to have more documentation.

In addition, I have concerns about the coding style. Some of the demo files do from PIL import Image, while others do import Image. Consistency is important in demo code, since users will copy from it while they are learning.

draw_text.py will throw an Exception with no message on any platform other than "darwin" or "win32", so I don't believe you when you say these demos pass on Ubuntu.

BLACK = "#ffffff"
WHITE = "#000000"

fg_color = WHITE

Choose a reason for hiding this comment

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

This variable is never used. Why does it exist?

Copy link
Author

Choose a reason for hiding this comment

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

It has deleted in new commit.

@inklesspen
Copy link

You should probably also have done a rebase so that the lena image isn't first checked in and then removed.

@aclark4life
Copy link
Member

+1 to rebase

@ghost
Copy link
Author

ghost commented Jan 16, 2013

Thanks, can you fix "This pull request cannot be automatically merged." ?

Sorry, I don't know how to fix it

Brian Crowell and others added 24 commits January 16, 2013 13:47
Plus, TGA was eligible for a round-trip test in test_imagefile. It has one
now.
The EPS encoder wasn't part of Gohlke's test suite, so the previous "fixes"
there were only expected syntactic ones. This gives a cleaner fix to the
encoder.

The decoder doesn't work in round-trip due to a missing eps_decoder method
on the core module, but it's clear it worked at some point.
For some reason, the PCX codec round-trips now.
After adding all the encode() calls, the PDF plugin (and a few others)
became much harder to read. This should be much easier on the eyes.
Not too convinced of the size fix. While it works against my file, I'm not
sure someone would have accidentally been an index off and not noticed.
This test includes an XPM file with transparency.
This is unnecessary now.
This fixes a build warning on 64-bit machines.
...although, you have to turn on deprecation warnings specifically in order
to get them.
…hon 2.5 is no longer supported (its support is broken in many ways in this branch); remove bundled doctest.py module (it is in stdlib since forever); remove extra stuff from tox.ini
String exceptions don't work anymore.
The Sane documentation seems to imply that these option strings contain
Latin-1 text, not byte data, so we decode it and present it to the user
that way.
@d-schmidt
Copy link
Contributor

There is still some work which need to be done before this is main-repo-ready.
Did you test it with Python 3 already?

histogram.py e.g. still contains .draft(), pixsel, xrange()

@aclark4life
Copy link
Member

Sorry, I don't know what's going on with this, but I'm going to close it until after Pillow 2.0 is released. I want lena demos, but I don't want to merge controversial code. 🎱

@aclark4life aclark4life closed this Mar 5, 2013
radarhere pushed a commit that referenced this pull request Dec 24, 2019
radarhere pushed a commit to radarhere/Pillow that referenced this pull request Sep 24, 2023
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.