Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Auto-layout mode that prevents plots from shrinking beyond readability #4234

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

BioTurboNick
Copy link
Member

This is a first pass that probably doesn't account for everything needed.

However, what it does is it simply computes a row and column count, the widths and sizes of them, passes the work down to the normal layout method, then the calling plot code reads the height and width attributes from the layout and overrides size if the total size needed is larger than the set size.

plot([plot(rand(3)) for x ∈ 1:20]..., layout = :auto, legend = :none)

Current:
image

This PR:
image

@BioTurboNick
Copy link
Member Author

Currently has a bias for taller over wider, but I can change that.

@codecov
Copy link

codecov bot commented Jun 10, 2022

Codecov Report

Merging #4234 (5fafc16) into master (6266ff8) will decrease coverage by 0.34%.
The diff coverage is 3.03%.

@@            Coverage Diff             @@
##           master    #4234      +/-   ##
==========================================
- Coverage   77.93%   77.58%   -0.35%     
==========================================
  Files          28       28              
  Lines        7101     7134      +33     
==========================================
+ Hits         5534     5535       +1     
- Misses       1567     1599      +32     
Impacted Files Coverage Δ
src/args.jl 74.47% <ø> (ø)
src/layouts.jl 70.57% <0.00%> (-5.83%) ⬇️
src/plot.jl 71.53% <20.00%> (-1.96%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6266ff8...5fafc16. Read the comment docs.

@@ -434,6 +434,7 @@ const _plot_defaults = KW(
:thickness_scaling => 1,
:display_type => :auto,
:warn_on_unsupported => true,
:plotarea => :auto,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We already have a notion for plotarea with a slightly different meaning

plotarea::BoundingBox # the part where the data goes

IIIUC, your plotarea is the whole area covered by one subplot, whereas the subplot field is the area of the axes and data without labels and margins and stuff

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah hmm. What would you recommend?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

call it subplotarea would be one option. This would also need an entry in arg_desc.jl

@BeastyBlacksmith
Copy link
Member

In general this is a different thing, than I had in mind with size = :auto from our conversation on slack, but having a way to say "I don't care about the specific layout" seems useful to me nonetheless.

@BioTurboNick
Copy link
Member Author

Hmm ok. Two of my pain points I'd like to resolve is being able to specify rows/columns for a grid without having to provide an exact number of plots to fill it, and prioritizing visibility of many plots over a fixed overall plot area.

How can this fit in with what you have in mind, so that they don't conflict?

@BeastyBlacksmith
Copy link
Member

BeastyBlacksmith commented Jun 15, 2022

being able to specify rows/columns for a grid without having to provide an exact number of plots to fill it,

can you give an example of how you would use it, if it where implemented?

What I don't like here is that the plot size changes without the user explicitly requesting it.
Here is one idea how we could handle things:

  • layout = :auto creates a layout that fits all plots "optimally" in the given size
  • size = :auto indicates that the overall plot size is allowed to change

Then you could have an automatic layout that changes plot size with layout = :auto and size = :auto. It might also be an option to check whether we can use GridLayoutBase.jl.
We could also think about having more descriptive modes, than just :auto maybe things like :constplotarea, :raggedright, etc.

@BioTurboNick
Copy link
Member Author

being able to specify rows/columns for a grid without having to provide an exact number of plots to fill it,

can you give an example of how you would use it, if it where implemented?

I'm building Pluto notebooks for our analysis pipeline, and there can be a variable number of samples per experiment. I may know that I can comfortably fit 3 plots across but otherwise don't care how many rows it takes. Or vice versa for columns. A more specific example might be a linear trace for each event. They can get pretty short and still be legible because the length matters more.

@BeastyBlacksmith
Copy link
Member

being able to specify rows/columns for a grid without having to provide an exact number of plots to fill it,

can you give an example of how you would use it, if it where implemented?

I'm building Pluto notebooks for our analysis pipeline, and there can be a variable number of samples per experiment. I may know that I can comfortably fit 3 plots across but otherwise don't care how many rows it takes. Or vice versa for columns. A more specific example might be a linear trace for each event. They can get pretty short and still be legible because the length matters more.

I meant in terms of function calls.

@BioTurboNick
Copy link
Member Author

I meant in terms of function calls.

Ah, yes. Maybe grid() could take a symbol as an argument, say :auto? Or we could have layout objects constructed with rows(x) and cols(x), which would be easier.

Maybe we would also want a minwidth and minheight provided that subplots would use to make sure they don't shrink below that.

Maybe size could take a symbol like :fit for the grow-to-fit behavior I've imagined here?

@t-bltg
Copy link
Member

t-bltg commented Nov 28, 2022

This won't be breaking would it ?

@BioTurboNick
Copy link
Member Author

I think perhaps this might be better considered as part of a revamped layout system for 2.0 than to solve here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants