On 07Apr2015 15:43, kai.pet...@gmail.com <kai.pet...@gmail.com> wrote:
I just wrote this bit (coming from Pascal) and am wondering how seasoned Python 
programmers would have done the same? Anything terribly non-python?

As always, thanks for all input.
K

"""
Creates a PNG image from EPD file
"""

import os, sys
from PIL import Image, ImageFont, ImageDraw

# -----------------------------------------------------------------------------
def RenderByte(draw, byte, x, y):
   blist = list(bin(byte).lstrip('0b')) # turn byte into list with 8 elements,

Remark: .lstrip does not do what you think.
You want:

   blist = list(bin(byte)[2:]) # turn byte into list with 8 elements,

to skip over the leading "0b". Otherwise, with lstrip, consider what would happen with a low value byte, with multiple leading zeroes. (Actually, on reflection, you might get away with it - but probably not, and anyway would be fragile against changing the ordering of the bits.)

I'd be popping off the least or most siginificant bit myself with "&" and "<<" or ">>". It might be wordier, bit it would be more in keeping with the actual bitwise operations going on.

   c = 0                                # each representing one bit
   for bit in blist:
       if bit:
           draw.point((x + c, y), fcolor)
       c += 1

This might be more Pythonic written:

 for c, bit in enumerate(blist):

which will emit (0, b0), (1, b1) and so forth for bit 0, bit 1 etc (where bit 0 is the leftmost from your list, etc). Avoids the "c = 0" and the "c += 1" and also makes your loop robust against adding control flows later which might skip the final "c += 1" at the end, even by accident.

   return

You generally do not need a trailing return with no value; it is implicit.

def EPD_2_Image(edpfilename, imagefilename):
   # get out of here if EPD file not present
   if not os.path.isfile(epdfilename):
       print 'file not found: ' + edpfilename
       return

Traditionally one would raise an exception instead of returning. Eg:

 if not os.path.isfile(epdfilename):
   raise ValueError("missing file: %r" % (epdfilename,))

and have the caller handle the issue. Similarly for all the other pre-checks below.

Also, it is normal for error messages to be directed to sys.stderr (this allows them to be logged independently of the program's normal output; for example the normal output might be sent to a file, but the error messages would continue to be displayed on screen). So were you to iussue a print statement (instead of an exception) you would write:

 print >>sys.stderr, ....

or in Python 3:

 print(...., file=sys.stderr)

   # is this a valid EPD file?
   filesize = os.path.getsize(epdfilename)
   if (((xdim / 8) * ydim) + header) <> filesize:
       print 'incorrect file size: ' + edpfilename
       return

   # blow old destination file away
   if os.path.isfile(imagefilename):
       print 'deleting old dest. file: ' + imagefilename
       os.remove(imagefilename)

   print 'processing...'

   # set up PIL objects
   img  = Image.new('1', (xdim, ydim), bcolor)   # '1' = Bi-tonal image
   draw = ImageDraw.Draw(img)

   # read entire EPD file into byte array (without the header)
   content = bytearray(open(epdfilename, 'rb').read())[16:]

   # image coord origin at top/left according to PIL documentation
   pos = 0
   for y in range(ydim):
       x = 0
       for byte in range(xdim / 8):   # 8 pixels 'stuffed' into one byte
           RenderByte(draw, content[pos], x, y)
           pos += 1
           x   += 8

   img.save(imagefilename)   # format is inferred from given extension
   print 'done.'

   return

Again, this "return" can be implicit.

# -----------------------------------------------------------------------------

xdim   = 1024
ydim   = 1280
header = 16
black  = 0
white  = 1
bcolor = black
fcolor = white

epdfilename   = 'c:\\temp\\drawtest2.epd'
imagefilename = 'c:\\temp\\drawtest2.png'

Normally these values would be at the top of your program, and named in UPPER_CASE to indicate that they are like "constants" in other languages. So you might put this:

 XDIM   = 1024
 YDIM   = 1280
 HEADER_SIZE = 16
 BLACK  = 0
 WHITE  = 1
 BCOLOR = BLACK
 FCOLOR = WHITE

 EPD_FILENAME   = 'c:\\temp\\drawtest2.epd'
 IMAGE_FILENAME = 'c:\\temp\\drawtest2.png'

at the top of the script.

I notice you have a bare [16:] in your "content =" line. Should that not say "[HEADER_SIZE:]" ?

EPD_2_Image(epdfilename, imagefilename)

I tend to write things like this as thought they could become python modules for reuse. (Often they do; why write something twice?)

So the base of the script becomes like this:

if __name__ == '__main__':
 EPD_2_Image(EPD_FILENAME, IMAGE_FILENAME)

In this way, when you invoke the .py file directly __name__ is "__main__" and your function gets run. But it you move this all into a module which may be imported, __name__ will be the module name, an so not invoke the main function. The importing code can then do so as it sees fit.

Cheers,
Cameron Simpson <c...@zip.com.au>

I heard a funny one this weekend.  I was belaying a friend on a very short
problem and when she was pumped out she told me to "Let me down" and my
other friend that was standing nearby said.  "You were never UP!".
       - Bryan Laws <bryanl...@aol.com>
--
https://mail.python.org/mailman/listinfo/python-list

Reply via email to