Great!  I am glad it was helpful. <div><br></div><div>Be Well</div><div>Anthony<br><br><div class="gmail_quote">On Fri, Aug 31, 2012 at 6:05 AM, Matthew Turk <span dir="ltr"><<a href="mailto:matthewturk@gmail.com" target="_blank">matthewturk@gmail.com</a>></span> wrote:<br>

<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">Hi Anthony,<br>
<br>
Thanks for your suggestions!  I've updated the pull request here:<br>
<br>
<a href="https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class" target="_blank">https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class</a><br>


<br>
and responded to your suggestions inline.<br>
<div class="im"><br>
On Wed, Aug 29, 2012 at 12:00 PM, Anthony Scopatz <<a href="mailto:scopatz@gmail.com">scopatz@gmail.com</a>> wrote:<br>
> Hello Matt,<br>
><br>
> Thanks for putting this together!  After briefly looking this over, it seems<br>
> like some methods which have "cartesian" in the name are slightly mis-named.<br>
> As implemented, cartesian_length() is really just a path_length().<br>
<br>
</div>Ah, alright.  I've changed the name to path_length, and cleaned up a<br>
few other methods.<br>
<div class="im"><br>
><br>
> I agree with the sentiment of the convert_to/from_cartesian() methods.<br>
> However, I think half of these could be redundant.  It basically depends on<br>
> whether you are choosing that Cartesian be the canonical form or not.  So I<br>
> see two strategies:<br>
><br>
> 1) All other systems have to/from cartesian methods.  Only need two methods<br>
> per coord system.  All coord transformations go through this cartesian.<br>
> (What you have, cleaner structure).<br>
><br>
> 2) All systems have either a to_<othersystem>() method.  One method per<br>
> conversion you wish to support.  Could be faster and have less floating<br>
> point error than #1.  This prevents the situation where to go from Polar -><br>
> Spherical you actually have to do Polar -> Cartesian -> Spherical.<br>
><br>
><br>
> The disadvantage of #2 is that you have to write more methods.  I guess<br>
> nothing is stopping you from implementing #1 and then just tacking on these<br>
> extra methods from #2 for cases where it makes sense.<br>
<br>
</div>On thinking about it, I like #2 better as well.  I'm going to start<br>
with just cylindrical and cartesian, and I've gone ahead and<br>
implemented the to/from methods between cartesian and cylindrical.<br>
<br>
If everyone likes this, I'll continue on adding fields for it and<br>
looking at how to integrate coordinate handling in a deeper way.  Let<br>
me know if there are any other suggestions.<br>
<br>
-Matt<br>
<div class="HOEnZb"><div class="h5"><br>
><br>
> My initial thoughts are that there are only 4 or 5 of these coordinate<br>
> systems so the number of permutations is not so high as to be unmanageable.<br>
> I also doubt that this is the kind of thing that users will be subclassing<br>
> and creating their own versions of.  So this is basically a "write once"<br>
> piece of code (::crosses fingers::).<br>
><br>
> In any event, I think what you have now is great.  It would just be nice to<br>
> circumvent going through cartesian when that is clearly too much work.<br>
><br>
> Be Well<br>
> Anthony<br>
><br>
> PS Sorry if this was rambling.  Must. find. coffee.<br>
><br>
> On Wed, Aug 29, 2012 at 10:27 AM, Matthew Turk <<a href="mailto:matthewturk@gmail.com">matthewturk@gmail.com</a>><br>
> wrote:<br>
>><br>
>> Hi Casey,<br>
>><br>
>> I've updated the PR:<br>
>><br>
>><br>
>> <a href="https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class" target="_blank">https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class</a><br>


>><br>
>> I think for now I'm going to leave the ABC in as a bookkeeping<br>
>> exercise while implementing the other handlers, and remove it later.<br>
>><br>
>> -Matt<br>
>><br>
>> On Wed, Aug 29, 2012 at 11:18 AM, Casey W. Stark <<a href="mailto:caseywstark@gmail.com">caseywstark@gmail.com</a>><br>
>> wrote:<br>
>> > Hey Matt.<br>
>> ><br>
>> > Sounds good. I would guess the dimensionality issue comes down to<br>
>> > implementation, but I think subclasses are fine.<br>
>> ><br>
>> > I am -1 on using abstract classes in general. Completely a style thing,<br>
>> > but<br>
>> > I think it ends up making things harder.<br>
>> ><br>
>> > - Casey<br>
>> ><br>
>> ><br>
>> > On Wed, Aug 29, 2012 at 7:52 AM, Matthew Turk <<a href="mailto:matthewturk@gmail.com">matthewturk@gmail.com</a>><br>
>> > wrote:<br>
>> >><br>
>> >> On Wed, Aug 29, 2012 at 10:43 AM, Casey W. Stark<br>
>> >> <<a href="mailto:caseywstark@gmail.com">caseywstark@gmail.com</a>><br>
>> >> wrote:<br>
>> >> > Hey Matt.<br>
>> >> ><br>
>> >> > I think this would be a big improvement, but I was wondering how it<br>
>> >> > interacts with other yt pieces. Does each output have geometry and<br>
>> >> > coordinate_handler objects as attributes?<br>
>> >><br>
>> >> Yup, that is the plan.  The idea is that we move all of the IO and<br>
>> >> particle/fluid selection into the geometry handler, and the handling<br>
>> >> of spatial layout to the coordinate handler.  This would mean, for<br>
>> >> instance, that we could push periodicity as well as path length into<br>
>> >> the coordinate handler; this would remove some of the need to<br>
>> >> constantly do wraparound checks and the like.<br>
>> >><br>
>> >> There is still somewhat the issue that *selection* of points to<br>
>> >> understand coordinate systems, which will require a bit more thought<br>
>> >> in the future but I think is still a tractable problem.<br>
>> >><br>
>> >> ><br>
>> >> > Is it possible to replace axis_name, axis_id, x_axis, and y_axis with<br>
>> >> > only<br>
>> >> > axis_names = ['x', ...]?<br>
>> >><br>
>> >> It is, but not with abc.abstractproperty.  (Initially I figured the<br>
>> >> cost of creating the dicts was low enough that we could do this to<br>
>> >> avoid worrying about mutable, class-level properties, but I think<br>
>> >> perhaps I like yours better.)  I'll remove some of the fancier<br>
>> >> ABC-stuff and slim it down.<br>
>> >><br>
>> >> ><br>
>> >> > - Casey<br>
>> >> ><br>
>> >> ><br>
>> >> > On Wed, Aug 29, 2012 at 7:28 AM, Matthew Turk <<a href="mailto:matthewturk@gmail.com">matthewturk@gmail.com</a>><br>
>> >> > wrote:<br>
>> >> >><br>
>> >> >> Hi all,<br>
>> >> >><br>
>> >> >> I've issued a pull request to the 3.0 repository, as I think it<br>
>> >> >> warrants discussion.  It's here:<br>
>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class" target="_blank">https://bitbucket.org/yt_analysis/yt-3.0/pull-request/5/initial-import-of-coordinate-handler-class</a><br>


>> >> >><br>
>> >> >> This includes a first pass at a coordinate handling system.  This is<br>
>> >> >> distinct from a geometry handling system; the coordinates here refer<br>
>> >> >> to how we handle coordinates and spatial locations internally,<br>
>> >> >> whereas<br>
>> >> >> geometry refers to how data is distributed throughout a domain and<br>
>> >> >> throughout places on disk.  For instance, coordinate handling would<br>
>> >> >> be<br>
>> >> >> cartesian, polar, spherical.<br>
>> >> >><br>
>> >> >> The reason I'm bringing it up for discussion is that I believe we<br>
>> >> >> want<br>
>> >> >> to move as much coordinate handling and transformation into a<br>
>> >> >> separate, well-defined class as possible.  Periodicity, distances<br>
>> >> >> and<br>
>> >> >> so on are all currently scattered throughout the code, and I'd like<br>
>> >> >> to<br>
>> >> >> try to consolidate them.  Additionally, as new coordinate systems<br>
>> >> >> (polar, spherical) are added, we'll need clear ways to delegate<br>
>> >> >> responsibility for things like "How do I calculate path length as I<br>
>> >> >> integrate?" or "What's the way to turn this into an image?"  I<br>
>> >> >> believe<br>
>> >> >> the best way to do that is to attach a coordinate system to the<br>
>> >> >> dataset object itself.  (We now have a polar pixelizer<br>
>> >> >> <a href="http://i.imgur.com/a4UGg.png" target="_blank">http://i.imgur.com/a4UGg.png</a> !)<br>
>> >> >><br>
>> >> >> The interface is currently set such that you need to define these<br>
>> >> >> methods and properties in order to implement a coordinate system:<br>
>> >> >><br>
>> >> >> coordinate_fields (this may go away, but it's for the analogs of<br>
>> >> >> 'x',<br>
>> >> >> 'y', 'z', as well as volume)<br>
>> >> >> pixelize<br>
>> >> >> convert_from_cartesian<br>
>> >> >> convert_to_cartesian<br>
>> >> >> axis_name<br>
>> >> >> axis_id<br>
>> >> >> x_axis<br>
>> >> >> y_axis<br>
>> >> >> period<br>
>> >> >><br>
>> >> >> Some of these currently live in dictionaries in<br>
>> >> >> yt/utilities/definitions.py, which is pretty sub-optimal.  I'd like<br>
>> >> >> to<br>
>> >> >> ask for feedback:<br>
>> >> >><br>
>> >> >> 1) Do these methods sufficiently cover everything we need to know in<br>
>> >> >> yt about a coordinate system?  Should any be added?<br>
>> >> >> 2) Do we need to directly address dimensionality as a separate<br>
>> >> >> subclass?<br>
>> >> >> 3) Should any of these be removed?<br>
>> >> >><br>
>> >> >> This will also help address these issues:<br>
>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt/issue/418/use-a-right-handed-coordinate-system" target="_blank">https://bitbucket.org/yt_analysis/yt/issue/418/use-a-right-handed-coordinate-system</a><br>


>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt/issue/422/ray-casting-in-cylindrical-coordinates" target="_blank">https://bitbucket.org/yt_analysis/yt/issue/422/ray-casting-in-cylindrical-coordinates</a><br>


>> >> >><br>
>> >> >><br>
>> >> >><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt/issue/421/refactor-non-cartesian-geometry" target="_blank">https://bitbucket.org/yt_analysis/yt/issue/421/refactor-non-cartesian-geometry</a><br>
>> >> >><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt/issue/345/non-cartesian-geometry" target="_blank">https://bitbucket.org/yt_analysis/yt/issue/345/non-cartesian-geometry</a><br>
>> >> >> <a href="https://bitbucket.org/yt_analysis/yt/issue/205/periodicity" target="_blank">https://bitbucket.org/yt_analysis/yt/issue/205/periodicity</a><br>
>> >> >><br>
>> >> >> -Matt<br>
>> >> >> _______________________________________________<br>
>> >> >> yt-dev mailing list<br>
>> >> >> <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
>> >> >> <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
>> >> ><br>
>> >> ><br>
>> >> ><br>
>> >> > _______________________________________________<br>
>> >> > yt-dev mailing list<br>
>> >> > <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
>> >> > <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
>> >> ><br>
>> >> _______________________________________________<br>
>> >> yt-dev mailing list<br>
>> >> <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
>> >> <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
>> ><br>
>> ><br>
>> ><br>
>> > _______________________________________________<br>
>> > yt-dev mailing list<br>
>> > <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
>> > <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
>> ><br>
>> _______________________________________________<br>
>> yt-dev mailing list<br>
>> <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
>> <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
><br>
><br>
><br>
> _______________________________________________<br>
> yt-dev mailing list<br>
> <a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
> <a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
><br>
_______________________________________________<br>
yt-dev mailing list<br>
<a href="mailto:yt-dev@lists.spacepope.org">yt-dev@lists.spacepope.org</a><br>
<a href="http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org" target="_blank">http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org</a><br>
</div></div></blockquote></div><br></div>