Closed bdeket closed 4 years ago
The build fails because the legend code has changed to be more compact. It is simple to fix (just update the draw step files and the image), however before you do that, I would like to have a high-level discussion about the approach.
With the possibility to place the plot legend at the top or bottom, outside the plot, it makes sense to allow for a horizontal layout of the legend. My thought was to provide two options for a plot-legend-orientation
parameter: 'horizontal
and 'vertical
, with the assumption that the option will be used in combination with the legend anchor: for example, if a user has lots of functions to plot, and the horizontal layout would not fit on the top of the plot, so they would use a vertical layout on the left or right side.
The code you propose in this pull request allows placing the legend entries in columns and rows -- this does provide more flexibility, but also introduces new problems:
the code which creates the legend is more complex -- yes, the code is already written in this pull request, but I also consider the fact that its complexity will make it more difficult for future developers to work with it
it introduces new possibilities for user preferences (e.g. compact layout versus equal size for each entry). In the PR 58 test, I choose equal size for the height, because it seemed to me that the legend looks nicer with the samples I used.
there are other legend attributes which remain hard coded (for example the gap between entries, the size of the sample lines, etc). Exposing them to the user would make the code even more complex and inroduces the possibility of bugs with different parameter combinations.
So my preference for now to keep things simple and have just horizontal and vertical layouts, and wait to get some user feedback from their use.
Also, for more complex legends, I think the best option is to let the user to construct the plot legend themselves using the pict
package, since the two use cases that I am aware of would not be helped by any of these features anyway.
Here is a legend used in the paper that Ben linked (yellow highlight is mine):
And here is one where the legend displays data from the plot, but in an unusual format, and can also be dragged around to see what is underneath it (this is in an interactive application):
I didn't update the test file of the old PR that now fails because I wanted to first discuss this. I didn't want to implement yet another parameter so I wanted to find out what would be best: The wide or compact layout.
But I think that a horizontal layout needs an option to specify more than one line. Especially in the wide version, it will overflow quickly when on one line.
Did you have a chance to look at the changes up to commit f71e2b? In that one, the changes to the legend calculation are pretty minor. The added complexity is mostly a result from trying to make it compact.
I wanted to find out what would be best: The wide or compact layout.
I don't think either wide or compact would be best, and instead we would need to provide an additional option (which I would like to avoid).
I introduced the wide format when I added "picts" as labels and the entries could be different height. Based on my testing, it seemed to me that the "wide" format would be better. However, in your testing, it seems the compact format looks better. I think the compact format will look bad in vertical layout when there are multiple line type plots, as the distance between lines will be uneven.
But I think that a horizontal layout needs an option to specify more than one line. Especially in the wide version, it will overflow quickly when on one line.
If a user has their legend overflow when it is in horizontal layout, they can switch to vertical layout and anchor it to the left or right of the plot. If that does not suit them, they can write their own legend as a pict, or report an issue so we have a better understanding of their use case.
Once we get more feedback from users, will have a better understanding of what requirements are and what possible improvements we can make. I understand that, when asked about this, everyone will have an opinion, but I prefer to wait for users to request these features without being prompted, as those requests come from actual use cases. Otherwise we risk implementing features which make things more complicated and yet most users might still not find them sufficiently flexible for what they need.
Did you have a chance to look at the changes up to commit f71e2b?
Yes, I did look at it, and yes the changes are not too complex, but as per my points above, we are now starting to provide more and more customization options yet we don't really know what the users actually need.
My thinking is that there are two options:
compact
and equal-size
(wide is not a good name).
Possible implementation for changing the legend-layout. This adds the option to specify the rows or columns of the legend:
images with commit f71e2b It is following the current setup that all legend labels have the same size. This leads to a lot of lost space in my opinion.
I also looked into keeping the maximum size per row/column with this result: images with commit f64587 (latest)
An automatic option to choose the minimum amount of columns or rows would need a revisit of the legend-margin calculation...