[yt-dev] Small adjustment for the PhasePlot API

[John ZuHone]

+1

On Jul 2, 2014, at 7:50 AM, Matthew Turk <matthewturk at gmail.com> wrote:

> I think this is a good change; I think it was always the intention to
> have parity between ProfilePlot and PhasePlot in this particular
> regard.
> 
> On Wed, Jul 2, 2014 at 12:42 AM, Britton Smith <brittonsmith at gmail.com> wrote:
>> +1
>> 
>> 
>> On Tue, Jul 1, 2014 at 9:53 PM, Cameron Hummels <chummels at gmail.com> wrote:
>>> 
>>> +1
>>> 
>>> 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.
>>> 
>>> 
>>> On Tue, Jul 1, 2014 at 4:08 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>>> wrote:
>>>> 
>>>> Hi all,
>>>> 
>>>> 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.
>>>> 
>>>> 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:
>>>> 
>>>> ph = PhasePlot(dd, 'density', 'temperature', ['cell_mass',
>>>> 'cell_volume'])
>>>> 
>>>> The second is via the profile keyword argument:
>>>> 
>>>> profile = create_profile(dd, ['density', 'temperature'], ['cell_mass',
>>>> 'cell_volume'])
>>>> ph = PhasePlot(source, None, None, None, profile=profile)
>>>> 
>>>> 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).
>>>> 
>>>> 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.
>>>> 
>>>> 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.
>>>> 
>>>> 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.
>>>> 
>>>> Thanks for your advice!
>>>> 
>>>> -Nathan
>>>> 
>>>> 
>>>> _______________________________________________
>>>> yt-dev mailing list
>>>> yt-dev at lists.spacepope.org
>>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>>> 
>>> 
>>> 
>>> 
>>> --
>>> Cameron Hummels
>>> Postdoctoral Researcher
>>> Steward Observatory
>>> University of Arizona
>>> http://chummels.org
>>> 
>>> _______________________________________________
>>> yt-dev mailing list
>>> yt-dev at lists.spacepope.org
>>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>>> 
>> 
>> 
>> _______________________________________________
>> yt-dev mailing list
>> yt-dev at lists.spacepope.org
>> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org
>> 
> _______________________________________________
> yt-dev mailing list
> yt-dev at lists.spacepope.org
> http://lists.spacepope.org/listinfo.cgi/yt-dev-spacepope.org