how to properly read a PPM file using Python

Question

Here is my overall instructions

Write a Color class that represents an RGB color using integer values in the range 0 to 255. Your class must: Be placed in image.py Provide a constructor that accepts the values of the red, green, and blue channels from the client and stores those values Provide public methods that return the values of the red, green, and blue channels

Write a PortablePixmap class that represents a PPM image. Your class must: Be placed in image.py Provide a constructor that accepts the magic number, width, height, maximum color value, and pixel data from the client and stores those values Store the pixel data as a list of (or list of lists of) Color objects Provide a public method that returns a string representation of the PPM image

Write a read_ppm function that opens a PPM image file, reads its contents, and returns a PortablePixmap object that holds its contents. Your function must: Be placed in image.py Read the contents of a PPM image file Not be sensitive to the formatting of the PPM image file Exit with an error if the numbers of expected and provided pixels differ

Write a main function that tests your read_ppm function. Your function must be placed in main.py

this is what I have thus far

class Color:
# constructor takes in values from client and stores them
def __init__(self, red, green, blue): 


    # checks that type of arg == int: raises exception otherwise 
    if (isinstance(red, int) and isinstance(green, int) and isinstance(blue, int)):     
        print("good stuff, indeed integers")
    else:   
        raise TypeError("Argument must be an integer.")

    # checks if values are between 0 and 225 
    if red < 0 or red > 225: 
        print("0 < rgb values < 225")
    elif green < 0 or green > 225:
        print("0 < rgb values < 225") 
    elif blue < 0 or blue > 225:
        print("0 < rgb values < 225")

    # instance variables (RGB values)
    self._red = red 
    self._green = green
    self._blue = blue 


# methods that reuturn RGB values
def returnRed(self): 
    return self._red 

def returnGreen(self):
    return self._green

def returnBlue(self):
    return self._blue


'''class that represents a PPM image'''
class PortablePixmap:
    def __init__(self, magic_number, width, height, max_color_value, pixel_data):
        self._magic_number = magic_number
        self._width = width
        self._height = height
        self._max_color_value = max_color_value
        self._pixel_data = pixel_data


    def __str__(self):
        s = self._magic_number
        s += '\n' + str(self._width)
        s += ' ' + str(self._height)
        s += '\n' + str(self._max_color_value)
        for pixel in self._pixel_data:
            s += ' ' + str(pixel[0])
            s += ' ' + str(pixel[1])
            s += ' ' + str(pixel[2])

        return s

I have a few questions for clarification.. 1. Did I go about creating the Color class correctly? 2. Do I even need to raise any exceptions in that class specifically? We will ultimately be reading from a file that contains everything in order but not necessarily on it's own individual line.

I really just want to know if I am going about this correctly. The instructions seem stepwise, but I am not really understanding how everything connects so I'm afraid I am either doing too much or too little.

Thanks in advance

Answer 1

It is not clear from the specification that you need to check the values, and your checks only raise exceptions in some cases, otherwise cause side effects (printing); from a reuse perspective, I'd prefer to have only the exceptions if any. Aside from the indentation error (which I assume is only here, not in your source) the Color class looks to cover the demands, although they are quite unpythonic with the accessors; probably someone was trained by Java.

The docstring should be inside the PortablePixmap class, not above it.

Most remarkable is the combination of demands that your class not be sensitive to the formatting of the PPM and store pixels as 8-bit unsigned RGB. This makes it impossible to support all PPMs, as they support 16-bit values (note the maxval field in the PPM format).

Your PortablePixmap class also doesn't use the Color class: "Store the pixel data as a list of (or list of lists of) Color objects". That requirement forces a rather awfully inefficient implementation, but the whole thing is an exercise, I suppose. You'll need to extract the RGB triplets from the pixel data string. That's also where you need the one check that is specified; verifying that there are exactly the right number of pixels. One would expect a ValueError exception if that fails.

If I were writing this sort of thing I might have used slots to reduce memory use for classes like Color, arrays to handle the large number of limited range numeric values, and possibly properties to make storage transparent without using unwieldy getter methods. split and join would make it easier to handle the collection of pixels.