Closed vasole closed 8 years ago
For the record, a few comments regarding the plot API (this does not change the need for refactoring and documentation of existing code):
I think the plane is the second option since the beginning, but the last option should be kept in mind to provide a more generic option if needed.
For longer term, a few links about a common plot API over multiple backends:
Well, it seems the only place at which there is no consensus is the DAU.
At the meeting it was pretty clear the different positions.
Yesterday I was at ID32. You should take a look at what has been developed there just "recycling" existing widgets. Nowhere was a complain about using an interface (API is a too heavy word in that context) to the plot different than matplotlib. Hey! They have to learn at minimum to instantiate an application, a widget, to show the widget and to start the application event loop. None of them are matplotlib commands. To learn a single command more (addCurve) to have everything they need for plotting curves or other one (addImage) for having everything they need for images does not seem a big price to be paid.
So, if the proposal is to keep a few commands for compatibility, to offer renamed ones for matplotlib compatibility while at the same time loosing the possibility to switch on-the-fly to a faster backend and not gaining anything over what it is existing, I think the answer is clear.
By the way, in the first of the links you supply, Almar gives you a clear hint ("We need a split"). Consider the proposed abstraction the user API in Almar's web page. One can spend as much time as desired developing a fully matplotlib-like interface just for at the end, having the only impact at the user side of changing basically one command. I think we have other priorities.
May be redundant, but some of my ideas:
For early review/, here is the refactoring of PyMca PlotWidget: https://github.com/t20100/silx/tree/plot It is a work in progress.
It aims at being compatible with PyMca's PlotWidget 'public API' as much as possible.
There is a script to play with it here: https://github.com/t20100/sandbox/blob/master/testPlotWidget.py It can use either silx (default) or PyMca (python testPlotWidget.py pymca).
The main difference is that it does not inherit from PluginLoader. To me this should be added to the toolbar providing plugins or on-demand as a composition of the Plot and the PluginLoader (rational: The plugins need the plot but not the otherway around). The second difference is that it does not provide the printGraph method. I would like to have it as a separated class and provided in the user interface in the PlotWindow. Other small API difference: addItem has an 'overlay' argument, get* methods to retrieve curve/image information return an additional element in the list they return (needed after splitting user provided info from plot parameters).
Here is my API review/ideas.
Keep a rather procedural API for the main things, but add OO API under it. Basically, make add* methods return an object instead of a string key (the object legend) as of now. At first, this object can provide information about the object (e.g., curve linestyle, user-defined information...). It can also be used to uniquely identify a plot object and thus remove* method boils down to: remove(item) without giving the kind of item no matter how it is stored internally. This paves the way for updating plot items through the returned plot objects, and adding listener on each plot object if needed. It allows to add some functionalities without adding methods to the main API (e.g., show/hide objects, update...)
A proof of concept: https://github.com/t20100/sandbox/tree/master/plot
Now only available for curve: Remove this feature? Or add hide for image, item, markers. If using object oriented plot objects, this can be available as a visible attribute of objects.
Now defined as a dict with 'name', 'normalization', 'autoscale', 'vmin', 'vmax'. vmin and vmax are ignored if autoscale is True. 'autoscale' is not necessary, it can be enable when 'vmin' and/or 'vmax' are None.
Some quick comments to explain the rationale behind some things and the proposed changes
The goal is to be able to pan and (less important) zoom while drawing a polygonal mask.
Just a comment.
I get a FutureWarning on line 126 of BackendMatplotlib.py
FutureWarning: "comparison to 'None' will result in an elementwise object comparison in the future"
Draw line raise NotImplementedError: Unsupported item shape line
Test error bars gives a warning: CANNOT CALCULATE LIMITS
For the plugins, it is lost for now, but can be composed with Plot class. I added a dummy backend to the Plot which allows to run the Plot headless.
Fixed the raised issues.
Discussion Introduction
Plot1D2D.pdf
Conclusion