9
\$\begingroup\$

I've done a script that will write things as images, which can then be read elsewhere (eg. you could share an mp3 through imgur). This is the first one I've done where I've been trying to improve my writing style and make stuff run as efficient as possible, so if there is anything you see here that I can improve it'd be really helpful to know.

For the record, this is an old version, but after this part it got a lot more complicated. I kept the same writing style anyway, so this is a lot easier to understand. This version will make a grey noisy image, but I added some code to use custom images after (which is where it got more complicated).

from PIL import Image
import cPickle, base64
class ImageStore:
 def __init__( self, imageName="ImageDataStore" ):
 self.imageDataPadding = [116, 64, 84, 123, 93, 73, 106]
 self.imageName = imageName
 def __repr__( self ):
 return "Use this to store or read data from an image.\nUsage:\n - ImageStore().write(input), ImageStore().read()\nYou can also define the name and location of the image.\n - ImageStore( 'C:\filename )'"
 def write( self, input, widthRatio=0.52 ):
 encodedData = base64.b64encode( cPickle.dumps( input ) )
 pixelData = [int( format( ord( letter ) ) ) for letter in encodedData]
 pixelData += imageDataPadding
 #Pad to end with multiple of 3
 for i in xrange( 3-len( pixelData ) ):
 rawData += [255]
 #Set image info
 minimumWidth = 8
 minimumHeight = 8
 currentPixels = len( pixelData )/3
 #Calculate width and height
 if currentPixels <= minimumWidth*minimumHeight:
 #Set to minimum values
 width = minimumWidth
 height = minimumHeight
 else:
 #Calculate based on ratio
 width = int( round( pow( currentPixels, ratioWidth ), -1 ) )
 #Make sure it is divisable by 3
 width /= 3
 width *= 3
 #Note: height is now calculated to be as small as possible while still containing all the data, this is just the old way
 height = int( round( pow( width, 1/( 1/( 1-ratioWidth )-1 ) ), -1 ) )
 #Draw image
 imageOutput = Image.new("RGB", ( width, height ) )
 imageData = imageOutput.load()
 #Assign pixel colours
 numbersToAddIncrement = 0
 for y in range( height ):
 for x in range( width ):
 try:
 for i in xrange( 3 ):
 dataRGB[i] = inputData[numbersToAddIncrement]
 numbersToAddIncrement += 1
 except:
 for i in xrange( 3 ):
 dataRGB[i] = rd.randint( 52, 128 )
 dataRGB = [number[1] for number in dataRGB.items()]
 imageData[x,y] = tuple( dataRGB )
 #Save image
 imageOutput.save( str( self.imageName ) + ".png", "PNG" )
 return "Saved file: " + str( self.imageName ) + ".png"
 def read( self ):
 #Read image
 try:
 imageInput = Image.open( str( self.imageName ) + ".png" )
 except:
 return "No image found."
 #Store pixel info
 rawData = []
 for pixels in imageInput.getdata():
 for rgb in xrange( 3 ):
 rawData.append( pixels[rgb] )
 #Truncate end of file
 try:
 for i in xrange( len( rawData ) ):
 j = 0
 while rawData[i+j] == imageDataPadding[j]:
 j += 1
 if j == len( imageDataPadding ):
 rawData = rawData[0:i]
 break
 if j == len( imageDataPadding ):
 break
 except:
 print "File is probably corrupted."
 #Decode data 
 encodedData = "".join( [chr( pixel ) for pixel in rawData] )
 outputData = cPickle.loads( base64.b64decode( encodedData ) )
 return outputData

Also, with custom images, I'm not quite happy with the way it works. Basically, I modify the pixels on an image to store the data, and to read it, you compare the modified image to the original one. Could anyone think of a way that wouldn't require the original image?

And here's the whole thing in it's current state (despite my best efforts it got messy) - http://pastebin.com/e1VUTPb5

200_success
145k22 gold badges190 silver badges478 bronze badges
asked Mar 6, 2015 at 18:28
\$\endgroup\$

1 Answer 1

7
\$\begingroup\$
  1. First off, you seem very confused with your whitespace. In Python, you shouldn't have "padding" around the contents of braces, {[()]}. For example, the line, xrange( 3-len( pixel_data ) ) would become xrange(3 - len(pixelData)).
  2. Secondly, you need whitespace between your operators. For example, len( pixelData )/3 would become len(pixel_data) / 3.
  3. Your naming violates the official style guidelines from PEP8. Variables and functions should be in snake_case, and classes should be in PascalCase.
  4. Never use except without knowing which specific error to catch. There could easily be an error that goes un-noticed because you weren't trying to catch a specific error.
  5. Finally, the code below the comment #Store pixel info, in the function read, can be shortened to this:

    rawData = [[pixels[rgb] for rgb in xrange(3)] for pixels in imageInput.getdata()]
    
answered Jun 29, 2015 at 22:05
\$\endgroup\$
1
  • \$\begingroup\$ Thanks, I've recently tried my best to sort out 1, 3 and 4, and with the whitespace between operators, I try to not use it unless it's a larger calculation where spaces make it easy to understand the different sections (simple example would be like (input**2)/5 + input/2 as opposed to input ** 2 / 5 + input / 2). It did dawn on me the #Store pixel info part seemed a bit clunky, so thanks haha. The code got up to 3500 lines with no documentation though, so instead of going through and updating it all, I'll probably try rewrite it from scratch :) \$\endgroup\$ Commented Jun 30, 2015 at 8:13

Your Answer

Draft saved
Draft discarded

Sign up or log in

Sign up using Google
Sign up using Email and Password

Post as a guest

Required, but never shown

Post as a guest

Required, but never shown

By clicking "Post Your Answer", you agree to our terms of service and acknowledge you have read our privacy policy.

Start asking to get answers

Find the answer to your question by asking.

Ask question

Explore related questions

See similar questions with these tags.