On Thu, Sep 27, 2012 at 8:18 PM, Todd <toddr...@gmail.com> wrote: > On Thu, Sep 27, 2012 at 1:44 PM, Todd <toddr...@gmail.com> wrote: >> On Thu, Sep 27, 2012 at 1:12 PM, Damon McDougall >> <damon.mcdoug...@gmail.com> wrote: >>> Hi Todd, >>> >>> Firstly, thanks for taking the time to crystallise your thoughts in >>> words first. This is one of my bad habits; I tend to rush into things. >>> >>> I have some feedback for you, hopefully we can all work together to >>> get this right. It's difficult when something new gets implemented and >>> someone expects it to do something and the only way to resolve it is >>> to break the calling API. >> >> Where is API broken? >> >>> Anyway, I hope you'll find my comments >>> helpful at the least. I also encourage others to weigh in with >>> opinions and ideas. >> >> Okay, I will discuss the rationale. I will snip out anything you >> agree on for brevity. >> >>>> Assuming we go with the name, here is my proposed call signature: >>>> >>>> EventRaster(x, offset=0, height=1, **kargs) >>> >>> CamelCase is discouraged for method names. Perhaps 'eventraster'? >> >> Fair enough. >> >>> Also, we could also let **kwargs swallow the 'offset' and 'height' >>> keyword arguments. Then the user isn't constrained by which order to >>> put them in. The downside of this approach is that introspection is >>> more difficult. >> >> I don't understand the advantage of this approach. If you use keyword >> arguments, then the order doesn't matter. So with the approach above >> you can use keyword arguments, in which case you can use whatever >> order they want, or you can use positional arguments. On the other >> hand putting it in **kwargs, we lose the ability to use positional >> arguments. So we lose nothing by allowing both positional and keyword >> arguments. It is also easier to implement. >> >>>> offset determines the positions of the rows. By default, the first >>>> row is placed with the line center y=0, and subsequent rows are placed >>>> with the line centers at increasing 1-unit increments. If offset is >>>> defined and is a scalar, the first row is placed at the offset, and >>>> subsequent rows are placed at increasing 1-unit increments. If offset >>>> is an array, it must be a 1D array of the same length as the second >>>> dimension of x. In this case each element in offset determines the >>>> center of the corresponding row in the plot. >>> >>> How about letting offset be either: a) a scalar, determining the >>> offset of all rows equally; or b) an array, determining the offset of >>> each row individually. >> >> Because people are almost never going to want to have all the lines >> stacked right on top of each other. The plot would be indecipherable >> that way. The defaults are chosen to handle the most common use-cases >> most easily. >> >>> In fact, why plot each row at integer y >>> coordinates? Could we allow for an optional y-coordinate array, each >>> element of which would be the y-coordinate at which to plot a row of >>> lines. Then you wouldn't need offset. >> >> That is exactly what offset does if you pass an array. >> >>>> If this is going to be used to implement rug plots, it would need some >>>> way to handle columns of horizontal lines in addition to rows of >>>> vertical lines. I see two ways to implement this. First is to have >>>> to plot types, perhaps HEventRaster and VEventRaster. The first would >>>> be as described above, while the second would be similar but >>>> everything rotated 90 degrees. Another possibility is to change the >>>> call signature to this: >>>> >>>> EventRaster(x, y=None, offset=0, height=1, **kargs) >>> >>> I think accepting an 'orientation' kwarg, which can take either >>> 'horizontal' or 'vertical', determining the orientation of the lines >>> and reversing the roles of the x and y arrays. >> >> That would work as well. Probably cleaner that way >> >>>> The function will return a list of a new collection type I am >>>> tentatively calling EventCollection. My thinking would be this would >>>> be a subclass of a new collection type called GenericLineCollection, >>>> which the current LineCollection would also subclass. They would >>>> share things like the color handling and segment handling, however the >>>> segment handling will be a "private" method that LineCollection will >>>> have a public wrapper for. On the other hand methods to set or add >>>> segments will remain private in EventCollection, although there will >>>> be a method to return the segments if an artist really wants to >>>> manipulate individual segments. >>> >>> Why can't we just use LineCollection? I don't see a good reason to >>> create a new collection class here; the plot is simple. >> >> Explained below. >> >>>> The reason for doing it this way is that manipulating individual rows >>>> of events should be very common, such as changing their position, >>>> color, marker, width, and so on. On the other hand manipulating lines >>>> individually should not be as common, although still supported. >>> >>> Fair enough, then maybe a better idea is to create your own >>> EventRaster class (note camel case) to hold all the relevant data in >>> arrays. Then make a 'construct_raster' method could return a >>> LineCollection. Then again, weren't you passing extra kwargs to >>> 'plot'? This approach would surely use ax.add_lines or >>> ax.add_linecollection something (I can't remember what it's called). >> >> The whole point of creating a new collection type is that artists will >> be able to manipulate individual sets of events. >> >> For example, with an ordinary LineCollection it will be extremely >> difficult to change the marker type, since doing so will change the >> marker for all 3 points on each segment rather than just the middle >> point. So if someone makes the plot, than wants to set rows to have >> different marker types instead of being lines, they can do that if we >> use a new collection class. But if we use LineCollection this becomes >> much more difficult. >> >> Similarly, with a LineCollection the lines lose their status as >> objects with a single distinct position. They become objects with 3 >> 2D coordinates. So if someone wants to add more events to the end, >> they need to take care of handling the x and y coordinates, making >> sure the x coordinates are the same and taking the y coordinates from >> one of the existing lines. Similarly changing the height or vertical >> position of all the objects is complicated, having to manually >> calculate and modify the y coordinates of each point in each segment. >> >> Again, the idea here is to make the most common use-cases as easy as >> possible. LineCollection objects aren't really suited to the sort of >> artistic changes that are typical with this sort of plot. >> >> In fact I would say that having a separate collection class is central >> to this idea. If users aren't able to manipulate the set of events as >> such after they create the plot, then there really isn't any advantage >> over just using a vlines plot. Calculating the ymin and ymax is one >> line of code each, it is the artistic changes that save many lines of >> code and a lot of complexity. > > I would also like to add that the new collection object would be > useful outside of this plot type. > > For example if someone wanted to create a rug plot as Nathaniel > described, and they want those along the same axes as the main plot, > then they would most likely not be be using the plot function, but > rather creating two individual collection objects in an existing > figure. > > I can imagine other cases besides a strict rug plot where adding such > a collection object could be useful.
Now that 1.2 is out, can we revisit this? I would like to get it implemented for the next feature release. -Todd ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://p.sf.net/sfu/appdyn_d2d_nov _______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel
