This patch looks mostly alright to me -- thanks very much for getting rid of the temporary file, and I agree that the FOF code should have a suffix to distinguish it from HOP structures. Here are some very minor comments... http://codereview.appspot.com/52058/diff/1/2 File yt/lagos/HaloFinding.py (right): http://codereview.appspot.com/52058/diff/1/2#newcode175 Line 175: _fields = ["particle_position_%s" % ax for ax in 'xyz',"particle_velocity_%s" % ax for ax in 'xyz'] This line is too long, split it to the next one. http://codereview.appspot.com/52058/diff/1/2#newcode404 Line 404: self.data_source.get_data(["particle_velocity_%s" % ax for ax in 'xyz']) This is redundant with the line above; additionally, we don't need particle_velocity until the write_out. http://codereview.appspot.com/52058/diff/1/5 File yt/lagos/fof/kd.h (right): http://codereview.appspot.com/52058/diff/1/5#newcode38 Line 38: } KDFOFNFOF; I think this should be KDNFOF; maybe the search-n-replace was done out of order? http://codereview.appspot.com/52058
participants (1)
-
matthewturk@gmail.com