arviz-devs / arviz-plots

ArviZ modular plotting
https://arviz-plots.readthedocs.io
Apache License 2.0
2 stars 1 forks source link

Handling of automatic sizing for figures. text and lines #65

Open OriolAbril opened 1 month ago

OriolAbril commented 1 month ago

We have some functionality to automatically set the size of the generated figure as well as plot properties like the linewidth or the fontsize. However, I am not sure it is worth the problems and complexity it introduces.

First and foremost, as all the code is common among all backends (save for what is in the backend/xyz folder) that means each backend needs to implement the functions used for setting these properties.

In matplotlib it is relatively straightforward, as we can get most everything either from rcParams or from figure/axes methods. It is also deciptively straightforward though because for fontsize for example, there is a base property (which we currently use) but then each element has its own default fontsize (titles, ticklabels, axis labels, annotation, legend entries...) and those can be set relative to the base property, or they can be set as absolute values. Thus, we are adapting some properties to the size of the generated figure, but we are also ignoring/breaking all these specific defaults that the theme being used by the user or the user themself might have set expecting it to be used yet it it completely ignored.

In bokeh it is a bit harder because most of these properties are not set. The current implementation/workaround is going over each plot and adding their widths/heights to get the total chart size. Thus, what was one line in matplotlib becomes 17 lines: https://github.com/arviz-devs/arviz-plots/blob/main/src/arviz_plots/backend/bokeh/__init__.py#L100. And that already is ignoring the spacing between plots. Other related methods have similar issues.

In #61 I am doing some tests with plotly, and it seems even harder to do because as far as I can tell, there are chart level width and height properties, but unless they are set explicitly by the user when creating the object, they are None until the visualization is rendered. If we wanted to support user generated objects (which I think we'd want to) then we would need a failsafe for cases when those properties have not been set at creation time. And given in that situation there is no information available the failsafe would have to be don't set/scale any of these properties or use hardcoded values.


One possible alternative to this would be making it easy/easier for users to define these properties so if they generate a plot and it is too small, or the text is too big they can easily generate it again updating those properties. Then getting rid of this functionality.

Another alternative would be to try and define an "effort threshold" so that if implementing these methods requires too much work (which long term will probably mean more maintenance too) we instead use functions that return hardcoded values to use as defaults.

aloctavodia commented 1 month ago

I agree this can be problematic, it is probably better to remove it, at least until we think of a better alternative.

OriolAbril commented 1 month ago

I think the best way to go is probably to size charts (matplotlib figures) automatically, but don't try to rescale the text/lines in either case which is what gets more complicated.

For chart sizing we only need number of rows and columns, then we define a "good looking" chart size. Sizing charts at creation time works quite well and is quite consistent across backends already. So I don't think we need to find a better alternative for that.

However, for the rest, we need to first get the chart size (which is problematic), then get the library (or theme) default textsize and linewidth (also problematic) and finally rescale those textsizes and linewidth so they are a better match for the chart. Later on, if we find good ways to overcome these two main issues consistenly across backends, we can add the text/line sizing back.

What do you think? There will probably be cases where for large charts the text is to small and/or the lines too thin, but both should be quite straightforward to change. Moreover, both those properties are part of the common interface and changing them using plot_kwargs would be independent of the backend. We could add a "Customizing charts" page or something like that covering this to the docs, similar to https://python.arviz.org/en/stable/user_guide/plotting_with_matplotlib.html and https://python.arviz.org/en/stable/user_guide/plotting_with_bokeh.html but backend agnostic.