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

add backend and update_state kwargs to show #4558

Open
wants to merge 7 commits into
base: master
Choose a base branch
from

Conversation

bjarthur
Copy link
Contributor

@bjarthur bjarthur commented Nov 1, 2024

as discussed show is the way to "save" a figure to an IOBuffer. but if the figure was made with say WGLMakie and you want to save as say PDF then it's a bit cumbersome because you have to CairoMakie.activate! and then WGLMakie.activate! after showing. this PR make it more concise by adding a backend kwarg to show, just like save has one already. so one can simply show(...; backend=CairoMakie) instead of activating and deactivating it.

Type of change

Delete options that do not apply:

  • New feature (non-breaking change which adds functionality)

Checklist

  • Added an entry in CHANGELOG.md (for new features and breaking changes)
  • Added unit tests for new algorithms, conversion methods, etc.

@bjarthur
Copy link
Contributor Author

bjarthur commented Nov 1, 2024

do any other show methods defined by Makie need to have a backend kwarg added as well?

also, i noticed that show is not tested at all. should i bother to add one?

@SimonDanisch
Copy link
Member

Thank you, seems reasonable!

should i bother to add one?

Yeah why not :)

@bjarthur
Copy link
Contributor Author

bjarthur commented Nov 4, 2024

all i can think to test is for the presence of a backend kwarg to show.

ideally i'd like to test that specifying a different backend works, but that would require putting the test in one of the backend repos and adding a dependency to one of the other backends. you don't want to go that far, do you?

@bjarthur bjarthur marked this pull request as ready for review November 20, 2024 17:54
@bjarthur
Copy link
Contributor Author

ready for review

@bjarthur
Copy link
Contributor Author

the failed test on WGLMakie for julia 1.6 is a memory leak. is that related to this PR?

@bjarthur bjarthur changed the title add backend kwarg to show add backend and update_state kwargs to show Nov 20, 2024
@bjarthur
Copy link
Contributor Author

bjarthur commented Nov 20, 2024

just added another kwarg to make reseting the axis limits optional when show'ing.

[EDIT: let me know if you'd prefer this to be in a separate PR]

@@ -244,17 +244,16 @@ function Base.show(io::IO, m::MIME"text/markdown", fig::FigureLike)
throw(MethodError(show, io, m, fig))
end

function Base.show(io::IO, m::MIME, figlike::FigureLike)
function Base.show(io::IO, m::MIME, figlike::FigureLike; backend = current_backend(), update_state=true)
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think just update would be better since that's what we use in display()

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

Successfully merging this pull request may close these issues.

3 participants