<div dir="ltr">+1<div><br></div><div>I think this will make it cleaner, and 3.0 is where we break the API, so I don't think it's a huge problem since it's already being changed from 2.x.</div></div><div class="gmail_extra">

<br><br><div class="gmail_quote">On Tue, Jul 1, 2014 at 4:08 PM, Nathan Goldbaum <span dir="ltr"><<a href="mailto:nathan12343@gmail.com" target="_blank">nathan12343@gmail.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">

<div dir="ltr">Hi all,<div><br></div><div>There's a minor issue with the PhasePlot class that I'd like to fix - unfortunately fixing it implies an API change.  Before I issue a pull request I'd like to ask the group what they think.</div>



<div><br></div><div>Right now a PhasePlot can be created in two ways.  Both are via the PhasePlot constructor.  The first uses what we have in the docs:</div><div><br></div><div>ph = PhasePlot(dd, 'density', 'temperature', ['cell_mass', 'cell_volume'])</div>



<div><br></div><div>The second is via the profile keyword argument:</div><div><br></div><div>profile = create_profile(dd, ['density', 'temperature'], ['cell_mass', 'cell_volume'])</div><div>



ph = PhasePlot(source, None, None, None, profile=profile)</div><div><br></div><div>As you can see, the second approach is not so nice right now due to the positional arguments in the PhasePlot initializer (I'm passing None where I can pass dummy arguments -- I could pass anything in these slots, they are ignored by the PhasePlot initializer).</div>



<div><br></div><div>This would make a lot more sense if PhasePlot didn't have positional arguments and you could just do PhasePlot(profile=profile).  Then again, I'd prefer not to change the API so drastically and add possible headaches for dealing with the two ways the PhasePlot initializer can be used.</div>



<div><br></div><div>For ProfilePlot, we don't have a profile keyword argument.  Instead there is a classmethod named from_profiles that creates a ProfilePlot without explicitly invoking the class initializer.</div><div>



<br></div><div>I'd like to add a from_profiles method to PhasePlot to help make this nicer.  I'd also like to remove the profile keyword argument, but wanted to check with the list first before opening a PR that breaks the PhasePlot API.  I could also leave the profile keyword argument, but for the sake of simplicity I'd prefer if from_profiles were the only way to create a PhasePlot from a profile object.</div>



<div><br></div><div>Thanks for your advice!</div><span class="HOEnZb"><font color="#888888"><div><br></div><div>-Nathan</div><div><br></div></font></span></div>
<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></blockquote></div><br><br clear="all"><div><br></div>-- <br>Cameron Hummels<div>Postdoctoral Researcher</div><div>Steward Observatory</div><div>University of Arizona</div><div><a href="http://chummels.org" target="_blank">http://chummels.org</a></div>


</div>