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

Patchwork extension #95

Merged
merged 7 commits into from
Apr 27, 2024
Merged

Conversation

cnrrobertson
Copy link
Contributor

This PR extends the patchwork functionality from #61 for arbitrary numbers of GGPlots and GGPlotGrids. It adds tests and docs for these as well.

Although it's all working pretty well, there does seem to be one hiccup that I'm not sure how to overcome. In summary, we can do more than two operations with + simultaneously but not with | or /. i.e. plot + plot + plot gives 3 side by side plots of equal width whereas plot | plot | plot has two of 1/4 width and one of 1/2. This is effectively because the + operator works with variable number of arguments while the |, / process one by one. I'm not sure how to get around this as it has to do with how the operator expression is parsed.

You can explore this by looking at the difference between Meta.parse("1 + 2 + 3").args and Meta.parse("1 | 2 | 3").args.

- required specifying exact types accepted for `addplots.jl` to avoid
MethodErrors on dispatch
- doesn't work for more than 2 | operations instead needs to be written
as |(plot, plot, plot)
@cnrrobertson cnrrobertson mentioned this pull request Apr 26, 2024
3 tasks
@rdboyes
Copy link
Member

rdboyes commented Apr 26, 2024

I'm not 100% sure how to solve the problem, but maybe the answer is to manually set the colsizes to be equal in GridLayout's options? See here for accepted options: https://docs.makie.org/stable/explanations/specapi/#advanced_spec_layouting

@rdboyes
Copy link
Member

rdboyes commented Apr 26, 2024

The other thing - at least for /, this might be expected behaviour? I would have to go back and use R's patchwork, but a/b/c resulting in two quarter-height plots and one half-height plot feels correct to me. We could implement plot_layout in order to allow users to change that default once the configuration of the plots is correct

@cnrrobertson
Copy link
Contributor Author

Sorry, had some testing imports leftover that broke the Github Action. Should be ready now.

@rdboyes
Copy link
Member

rdboyes commented Apr 26, 2024

I’ll merge this on the weekend - apparently there are merge conflicts and I don’t want to try to resolve them on my phone!

@rdboyes rdboyes merged commit d561890 into TidierOrg:main Apr 27, 2024
5 checks passed
@cnrrobertson cnrrobertson deleted the patchwork_extension branch April 29, 2024 16:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants