Stjepan, Thank you for taking the time to review this library! Let me know when yours is in a stable state, and I'll respond in kind.
Hi Jake,
I finally got around to taking a look at your lib (although I used the code and docs from the sandbox - hopefully that's OK). First impressions - this functionality is extremely useful. Being able to make an .svg straight out of code makes creating plots so much easier, and the functionality you already have available allows for a lot of customization. And the potential for future growth of the lib is also exciting.
In exploring your lib, I first read the docs front to back. I may have skimmed through parts of it :-) Here are some comments (a lot of them are very nit-picky, or they are just regarding things that weren't perfectly clear).
* From the intro:
"The first task when dealing with numerical data is to plot it."
Ehm... Very motivational, but likely untrue? :-) I agree that plotting data is extremely useful and important, but I don't think it is necessarily always "the first task", or even a necessary task at all.
This is a vestigial statement from my initial proposal. I've had a few teachers (statisticians and mathematicians both) over the years say this as a mantra, but I agree with you that it's not always as imperative as I make it out to be :)
* From the Colors docs:
"The list contains one extra color element, blank, used when you need to pass a color, but would not like it to show up. This comes in handy for defining defaults for functions, for example."
I don't understand this. What do you mean by "would not like it to show up?" The element whose color you are specifying should not show up?
SVG is cascading, in the CSS sense. If we have the following SVG fragment: <g fill="red"> <g> <rect x="5" y="50" width="490" height="295"/> </g> </g> the rectangle produced will have a red fill. If you would like the rectangle to simply be a frame, you can provide the color "blank", and be written as the following: <g fill="red"> <g> <rect x="5" y="50" width="490" height="295" fill="none"/> </g> </g> and it will appear as a rectangular frame. This will be made clearer in the documentation.
* From the x and y axis documentation:
"For an alternate way to display a regular axis, you can use an external style:"
What is an external style?
http://tinyurl.com/2vzp2o [tcnj.edu] I will provide an example in 2D as well.
* About customizing the plot
It first struck me as odd that some drawing parameters are specified through the plot object, and some at plotting time (.plot method). Then I realized that it is because there are common "plot" elements - axis, title, background, etc., and then there are elements that are common to each range "plot" - area fill, bezier, line color...
First thing - how can we disambiguate between "plot" as "the whole shabang" / with options describing common supporting elements (axis, title, ...), and "plot" as what is visualizing a single range (drawn via .plot())?
One of my ideas for post-GSoC was to try to group similar style methods using Boost.Parameter: my_plot.x_axis_style( width = 2, color = red, position = left) .legend_style( font_size = 15, background_color = lightgray); To me, this helps the user understand at a glance what is happening, as well as allowing for some clarity as to what sections are being affected. I wasn't sure that I was going to go this route, but the more I look at it, the better a solution it appears to be. My only concern is that this is very non-standard syntax, but OTOH it's already accepted into Boost, so on average, the community has accepted this. This should help a little with the first part of your concern. However, the definition of series styles should be done at the time the series is entered into the plot, IMO. Users are much less likely to define colors for the wrong series if they do it when they call plot(). Changing the name of the plot function to something like draw_series could help, but I also think that brief familiarity of the library (plus differentiating plot as a noun and plot as a verb) makes this a non-issue.
Second - specifying the line / area fill characteristics (only) at .plot() invocation seems to have a drawback. Suppose that I want to create a large number of .SVGs, out of the same data but all in different styles. So I create the plot in the first style, and output it to a file. Now I want to change the style for the second file. For the background elements, i can just change them by invoking the appropriate methods. But for the .plots? There seems to be no mechanism for me to go in and change the style of a plot drawn with .plot(). So, I have to recreate each plot from scratch. Maybe an avenue for this will be added when you allow for the use of external stylesheets? Or maybe you will cover it as a part of "avoiding having to redraw the entire plot each time".
You don't have to redo the *entire* graph from scratch, but yes, there is currently no way to modify the style of a series that is already stored. The stylesheet method could be a potential solution to this, if I allow the user to define class and ID names at .plot() invocation. Something like the following is also possible: my_plot.series(3).style().fill_color(black).stroke_color(white);
* The defaults tables docs
The two tables have a huge overlap - could they be condensed into one table? There could be two columns, describing the setting for each scenario (1d/2d). if something has the same default value for both, it can span both columns, otherwise you can list different values in each column or say "N/A" in one of the columns.
I will have to play around with it and see what looks better from the viewpoint of looking up information. IMO, the biggest use of a table like you mention is to do feature comparisons. When I use reference documents, it is purely a lookup function. If a user is looking for data on a 1D plot, they don't need to have information about 2D plots in front of them.. it's just extra noise, as far as the person is concerned. They are performing purely a lookup task. This gets even more difficult when differentiating between the 1D and 2D plot() features, as they both use Boost.Parameter, and to different ends. I would either have separate sections that still have redundant information, or I would have to try to take extra care to not confuse the user between options available in 1D and 2D (which is a concern I have, since named parameters are relatively unheard of outside of Boost).
Also, it would be nice to cross-reference this with the functions that change the values.
I don't understand what you mean.
In the example you have:
class my_functor { typdef pair<double, double> result_type;
pair<double, double> operator()(const human& _hum) const { return pair<double, double>(i, _hum.age()); } }
- what is this "i"?
A vestige of attempting to make functors that store state. This example will probably be replaced with a solution using counting_iterator<double>, but at the time of documentation, I ran into last-minute problems.
-----
At this point I looked at the code a bit and decided to try solving a task - creating an .svg in which the text is wrapped in \tex{} so that I can load it with Inkscape, save it as a .eps, and then use it in a LaTeX document via psfrag (the purpose of using psfrag is to make the fonts and their sizes match the text). Looking ahead, since I can specify all the text verbatim there should be no problem putting everything in \tex{}. The only exception are tick labels, and that will be solved when you add custom axis lables support (about that - you should disambiguate between axis labels and axis tick labels). Another thought - if you are planning on tackling another format next, I vote for PS/EPS :-)
When I do tackle formats, PS/EPS and .PNG are my two choices, but this won't happen until well after GSoC. PS/EPS is by far the most requested image format to implement.
I first ran into problems trying out your examples - for one, there is no Jamfile in the example directory (but this not be a problem with the version in the vault). I added my own, but then found out that the rest of your build configuration files seem to be set as if your lib was located in boost-root. So I ended up replacing all of them by the time I got to bjam the examples successfully.
My apologies. The examples were just that: example files that I've made as I wrote documentation, and that work upon invocation of gcc -I $BOOST_ROOT -I ../../.. example.cpp, and I didn't realize that I should provide a Jamfile to run them.
And then - a lot of warnings and a few errors (GCC 4.0.1/darwin). Most of the warnings were regarding the order of initialization of members in the constructors.
The warnings come up on GCC 4.1.2 as well, and I'm going to work on reducing them soon. I work primarily in MSVC with compatibility testing done with GCC, and MSVC (incorrectly) doesn't give warnings about this on the max error level, so this has been a problem for a long time.
Then, some of the errors were (nothing built successfully for me):
1d_full_layout.cpp:82: instantiated from here ../../../boost/svg_plot/svg_1d_plot.hpp:127: error: 'is_limit' was not declared in this scope
2d_simple: ../../../boost/svg_plot/detail/numeric_limits_handling.hpp: In function 'bool boost::svg::detail::limit_NaN(double)': ../../../boost/svg_plot/detail/numeric_limits_handling.hpp:35: error: '_isnan' was not declared in this scope
I didn't look into the reasons for the errors much. Simple fix?
So simple that they've already been fixed :). You must have downloaded the library at a bad time, because I think the errors were only in the SVN repository for 30 minutes or so.
Well, that's it for now :-) I can go further after I get past the errors.
Cheers,
Stjepan
Thank you for the comments! Jake