[yt-dev] ellipsoid pull request suggestions from Matt

[Geoffrey So]

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.

* 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.

- What is a base class or subclasses in YT?  Where can I avoid the code
reuse, and override?

* 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.

- 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.

* 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.")}}}

- 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?
- I think I tried to write an exception but didn't quite understand how it
works, going to give it another try here

Do I replace

        if na.size(self["particle_position_x"]) < 4:
            print "not enough particles to form ellipsoid returning zeros"
            return (0, 0, 0, 0, 0, 0, 0)

with something like

        if na.size(self["particle_position_x"]) < 4:
            raise YTNotEnoughParticles(na.size(self["particle_position_x"]))

and in exceptions.py

        class YTNotEnoughParticles(YTException):
            def __init__(self, nparticles):
                self.nparticles = nparticles

            def __str__(self):
                return "There are only %i particles, not enough to form an
ellipsoid" % (self.nparticles)

From
G.S.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.spacepope.org/pipermail/yt-dev-spacepope.org/attachments/20120620/ab41e547/attachment.htm>