patternpythonMinor
Iterator over row and columns
Viewed 0 times
iteratorcolumnsandrowover
Problem
I am writing a simple raytracer just for fun. I want to iterate over rows, and each row must be an iterator over columns. A strategy I produced is the following:
Briefly stated:
I personally think that it's difficult to understand, so I was wondering
class ViewPlane(object):
def __init__(self, resolution, pixel_size, gamma):
self.resolution = resolution
self.pixel_size = pixel_size
self.gamma = gamma
def __iter__(self):
def genR():
class IterRow(object):
def __init__(self, plane, row):
self.plane = plane
self.row = row
def __iter__(self):
def genC():
for column in xrange(self.plane.resolution[1]):
origin = (self.plane.pixel_size *
(column - self.plane.resolution[0] / 2.0 + 0.5)
,
self.plane.pixel_size *
(self.row - self.plane.resolution[1] / 2.0 + 0.5)
,
100.0
)
yield ( Ray(origin = origin, direction = (0.0,0.0,-1.0)),
(self.row,column)
)
return
return genC()
def __str__(self):
return "Row iterator on row "+str(self.row)
for row in xrange(self.resolution[0]):
yield IterRow(self, row)
return genR()Briefly stated:
ViewPlane contains the pixel plane. This class implements __iter__ which defines and returns a generator function. This generator function yields instances of an internal class IterRow, which in turn has a __iter__ method that returns a generator iterating over the columns.I personally think that it's difficult to understand, so I was wondering
Solution
class Foo:
def __iter__(self):
def innergen():
yield something
return innergen()Is the same as:
class Foo:
def __iter__(self):
yield somethingIn other words, there is no need to define the generator, just yield the values.
Defining a class inside the method just serves to make things complicated and hard to read. Define the class outside of the method.
Having return at the end of a function doesn't do anything and makes it look like you don't know what you are doing. (I'm not saying that you don't, but whenever you include code with effect it raises a red flag. Did you think it do something?)
Unless you really need to access the data row by row I suggesting flattening the iterator.
Stylistically, I prefer to break expressions up rather then trying to split them across several lines as you've done. I think it produces less clutter.
Here is my resulting code:
class ViewPlane(object):
def __init__(self, resolution, pixel_size, gamma):
self.resolution = resolution
self.pixel_size = pixel_size
self.gamma = gamma
def __iter__(self):
for row in xrange(self.resolution[0]):
for column in xrange(self.resolution[1]):
origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
origin = (origin_x, origin_y, 100)
ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )
yield ray, (row, column)I'm not familiar enough with RayTracers to know but I question having the iterator over ViewPlane produce Ray objects. An iterator is for producing the contents of the object. Is a really contained inside the ViewPlane? Or should the iterator produce coordinates and let the caller construct the Ray objects?
EDIT
Here is a version that doesn't flatten the iterator:
class ViewPlane(object):
def __init__(self, resolution, pixel_size, gamma):
self.resolution = resolution
self.pixel_size = pixel_size
self.gamma = gamma
def row_iter(self, row):
for column in xrange(self.resolution[1]):
origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
origin = (origin_x, origin_y, 100)
ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )
yield ray, (row, column)
def __iter__(self):
for row in xrange(self.resolution[0]):
yield self.row_iter(row)Code Snippets
class Foo:
def __iter__(self):
def innergen():
yield something
return innergen()class Foo:
def __iter__(self):
yield somethingclass ViewPlane(object):
def __init__(self, resolution, pixel_size, gamma):
self.resolution = resolution
self.pixel_size = pixel_size
self.gamma = gamma
def __iter__(self):
for row in xrange(self.resolution[0]):
for column in xrange(self.resolution[1]):
origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
origin = (origin_x, origin_y, 100)
ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )
yield ray, (row, column)class ViewPlane(object):
def __init__(self, resolution, pixel_size, gamma):
self.resolution = resolution
self.pixel_size = pixel_size
self.gamma = gamma
def row_iter(self, row):
for column in xrange(self.resolution[1]):
origin_x = self.pixel_size * self.resolution[0] / 2.0 + 0.5
origin_y = self.pixel_size * self.resolution[1] / 2.0 + 0.5
origin = (origin_x, origin_y, 100)
ray = Ray(origin = origin, direction = (0.0, 0.0, -1.0) )
yield ray, (row, column)
def __iter__(self):
for row in xrange(self.resolution[0]):
yield self.row_iter(row)Context
StackExchange Code Review Q#2424, answer score: 4
Revisions (0)
No revisions yet.