Hi Matt, thanks for the suggestions you've made on the pull request, there's just a couple questions and technical terms that I didn't understand.<div><br></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">* I'd suggest you put all of the ellipsoidal functions in the base class instead of distributing throughout the subclasses. This avoids code reuse. You can then override them in parallelHF's halos.<br>
<br></span></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">- What is a base class or subclasses in YT? Where can I avoid the code reuse, and override?</span></div>
<div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">* Please move all of the rotation matrix functions into yt/utilities/math_utils.py and add docstrings. These can then be imported in both data_containers.py and the halo functions.<br>
</span></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px"><br></span></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">- I see inside math_utils.py already has an get_rotation_matrix at the end, which is probably the general case you're referring to earlier when we talked (not sure how to use that with the "self" in the input). I'll just stick in the less general case of RX, RY, RZ unless you think it's redundant.</span></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><span style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif;font-size:13px">* Instead of printing, I would suggest either raising an exception (which you can define in yt/exceptions.py) or issuing a warning with {{{mylog.warning("Not enough particles to determine ellipsoidal parameters.")}}}<br>
<br></span></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">- What is better? Should I do a warning message and let the user know I'm returning zeros for the ellipsoid parameters? Or raise an exception, then let the user decide what to do if an exception is raised?</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">- I think I tried to write an exception but didn't quite understand how it works, going to give it another try here</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">Do I replace</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><div>
if na.size(self["particle_position_x"]) < 4:</div><div> print "not enough particles to form ellipsoid returning zeros"</div><div> return (0, 0, 0, 0, 0, 0, 0)</div></span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">with something like</span></font></div>
<div><span class="Apple-style-span" style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif"><br></span></div><div><span class="Apple-style-span" style="border-collapse:collapse;color:rgb(34,34,34);font-family:arial,sans-serif"> if na.size(self["particle_position_x"]) < 4:</span></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"> raise YTNotEnoughParticles(na.size(self["particle_position_x"]))</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">and in exceptions.py</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"> </span></font>class YTNotEnoughParticles(YTException):</div>
<div> def __init__(self, nparticles):</div><div> self.nparticles = nparticles</div><div><br></div><div> def __str__(self):</div><div> return "There are only %i particles, not enough to form an ellipsoid" % (self.nparticles)</div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse"><br></span></font></div><div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">From</span></font></div>
<div><font class="Apple-style-span" color="#222222" face="arial, sans-serif"><span class="Apple-style-span" style="border-collapse:collapse">G.S.</span></font></div>