On 11/11/2012 11:51 PM, Todd wrote: > 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. >
Absolutely. I think the next step, once you have an implementation, would be to submit a pull request and we can all help with a review. (BTW -- feel free to submit pull requests at any point in a release cycle -- we have both a master and a maintenance branch, so we can work on new stuff and stable stuff at the same time). Mike ------------------------------------------------------------------------------ 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
