On Thu, Sep 27, 2012 at 12: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?
Nowhere. I wasn't implying you were breaking something. My point was that it's easy to add functionality but hard to remove it, therefore it's important to get it right from the outset. I'm sorry for the misunderstanding; I wasn't clear. > >> 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. -- Damon McDougall http://www.damon-is-a-geek.com B2.39 Mathematics Institute University of Warwick Coventry West Midlands CV4 7AL United Kingdom ------------------------------------------------------------------------------ Everyone hates slow websites. So do we. Make your web apps faster with AppDynamics Download AppDynamics Lite for free today: http://ad.doubleclick.net/clk;258768047;13503038;j? http://info.appdynamics.com/FreeJavaPerformanceDownload.html _______________________________________________ Matplotlib-devel mailing list Matplotlib-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/matplotlib-devel