[yt-dev] Removing the max_level keyword argument from ProjectionPlot

[Matthew Turk]

Hi Cameron, Dave and Nathan,

I was actually the driving force behind removing the argument, and in
fact, I'd like to be the driving force behind removing *most* of the
kwargs that delve into a lot of stateful and "magic" object creation.
(i.e., with the movie making, for instance.)  I think it leads to a
lot of leakiness -- we can't cover all possible use cases, but as we
creep along, adding new things, we keep finding new areas that we want
to cover.  So the number of keyword arguments grows.  But then, the
number of potential conflicts between them grows, as does the
duplication of code, and so on and so forth.  I guess what I'm saying
is, we have too many ways to do similar and identical things.  And a
lot of that can be traced back to mistakes I have made in the past.
Much of the yt API was not designed, it was grown -- and watching
other projects more closely over the last few years, I've come to
really understand better how to work on design, and forethought, and
trying to ensure a consistent experience.

With the plot window, we tried to strip it down; they'd make a few
well-meaning decisions about what can be done to make a plot, and if
that wasn't good enough, it would throw up its hands and say, okay,
you do it.  Specifically, I think this applies when we think about the
 ways of creating objects that plots are built from.

But ... Cameron, your comment about matplotlib made me think about
this a bit more.  I don't want to aspire to the style of API that
Matplotlib has; in particular, I don't want to aim for the large
number of kwargs, which I think tends to lead to some confusion at
times.  However, I was reminded of the talk at SciPy 2012 by the late
John Hunter, entitled, "Matplotlib: Lessons from Middle Age."  During
the talk, I remember being struck by what a marvelous distillation of
running a community project he had presented.

Here is that talk:  http://www.youtube.com/watch?v=e3lTby5RI54 (highly
recommended viewing)

One of the things I wrote down (I dug out my notebook for this email)
was "Solid API".  I even circled it.  Later on I wrote, "Get a
familiar, stable, user-facing API right away."  And when I saw that
tonight, I realized I was in the wrong on this one.  We should not be
removing this option, not because it belongs there, but because it's
something people rely upon.  And we have a responsibility not to break
things unless absolutely necessary -- and sometimes it is necessary.
But an improvement in elegance is not necessity.

So we can decide how one might deprecate something like this; perhaps
in the 3.0 branch, when we will have other different types of
upgrades, we can consider removing it.  But I was wrong to encourage
Nathan to remove it now.  In return, I think that what we as code
reviewers should be doing is carefully considering the design of APIs
that become exposed and that get built on.  Thanks for helping me
remember this.

-Matt

PS On a personal note, shortly after his talk I was able to find John
and sincerely thank him for his candid, thoughtful and earnest
presentation.  His talk really was the highlight of my trip to SciPy
2012, and I am so very grateful I was able to tell him so.



On Thu, May 9, 2013 at 5:33 PM, Cameron Hummels <chummels at gmail.com> wrote:
> I've used max_level coupled with annotate_grid (and it's own max_level) in
> the past to highlight different structures in projections.  I guess I don't
> see why we're removing functionality here.  You'd rather have all display
> flags associated with the different plot functions?  So in order to
> replicate the functionality i describe above, this would mean creating a
> cutting region with its own max_level on a given object for each level and
> then piping each to projectionplot?
>
> I guess I'm not entirely behind the goals you have set out, explicitly to
> "remove rarely-used kwargs" from the plotting functions.  Matplotlib has
> about 10 bajillion kwargs associated with its functions that aren't always
> used, but it's nice to have them around for when you do need them.  I don't
> see having a lot of them around as a hindrance, as long as you make the
> default behavior what one would expect.  Most users will never have to
> fiddle with them, but they may come in handy in rare circumstances.  If
> having them around prevents normal users from spending a few hours trying to
> make a workaround for something that once did exist, then I say leave them
> in.
>
> I'm -1 on removing this functionality from the plotting routines, but I'm
> still open to hearing other opinions on the matter.
>
>
> On Thu, May 9, 2013 at 5:07 PM, Matthew Turk <matthewturk at gmail.com> wrote:
>>
>> On Thu, May 9, 2013 at 5:05 PM, Nathan Goldbaum <nathan12343 at gmail.com>
>> wrote:
>> > Hi all,
>> >
>> > As part of the eternal journey that is making yt's plotting code as
>> > awesome
>> > as possible, we've endeavored to make the plotting code as flexible as
>> > it
>> > needs to be for quick use but simple enough that a use isn't overloaded
>> > by
>> > unnecessary or barely-used keyword arguments.
>> >
>> > Right now there is a pending PR to rework some of the plotting routines,
>> > adding a couple of new features, and, per the subject of this e-mail,
>> > removing the max_level keyword from ProjectionPlot.
>> >
>> > I could see how having max_level might be useful if projections are very
>> > slow for a user's dataset.  However, she should be able to get the exact
>> > same result by explicitly constructing a projection data object and then
>> > creating a plot for it using projection.to_pw().  I also think in
>> > practice
>> > max_level isn't used very often since projections are quite fast, even
>> > on
>> > large datasets.
>>
>> I'm strongly in favor of keeping the ProjectionPlot, SlicePlot, etc,
>> all focused on display-related items, rather than data-related items.
>> I feel like they should be a good way to make sensible default objects
>> and then detailed modifications of plots from them.  That's why we
>> have .to_pw(), right?  :)  But I would like to hear if anyone
>> disagrees and thinks we need to keep max_level.
>>
>> >
>> > Am I incorrect in that assessment?  Please let me know if you'd like to
>> > keep
>> > the max_level keyword around and I'll happily revert that part of the
>> > pull
>> > request.
>> >
>> > Thanks for your help,
>> >
>> > Nathan
>> >
>> > _______________________________________________
>> > 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
>
>
>
>
> --
> 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
>