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

Compatibility with Makie 0.21 #73

Merged
merged 23 commits into from
Sep 6, 2024
Merged

Conversation

ovanvincq
Copy link
Contributor

I made some modifications to make GridapMakie compatible with Makie 0.21 (but not with previous versions).

There is only one small drawback: I do not understand how to change axis type for a full recipe and one must specify axis=(type=Makie.Axis3,aspect=:data) when plotting 3D data.

@JordiManyer
Copy link
Member

Hi @ovanvincq , thank you for your contribution! Sorry it took so long to get a response, I was not aware of the PR...

Could you please pull the master branch to resolve the issues? Most of the changes you already apply, so I believe it will be quite straightforward.

@JordiManyer JordiManyer self-requested a review September 6, 2024 03:05
@JordiManyer
Copy link
Member

JordiManyer commented Sep 6, 2024

There is only one small drawback: I do not understand how to change axis type for a full recipe and one must specify axis=(type=Makie.Axis3,aspect=:data) when plotting 3D data.

I am definitely not an expert, but browsing through the docs I found this:

Makie.preferred_axis_type(plot::MyPlot) = Makie.LScene # if you need the entire plot object as information

and from the Axis3 docs, I think we might be able to give it the aspect kwarg as

Axis3(aspect = :data)

could you try it out @ovanvincq ?

@ovanvincq
Copy link
Contributor Author

I am definitely not an expert, but browsing through the docs I found this:

Makie.preferred_axis_type(plot::MyPlot) = Makie.LScene # if you need the entire plot object as information

@JordiManyer Wonderful! I I commit the changes to take your advice into account and it seems that all the examples (in runtest.jl and README.md) work.

@JordiManyer
Copy link
Member

Ok great! I've resolved the conflicts with master. Once the test pass I'll merge. Thank you again for your time and work!

@JordiManyer JordiManyer merged commit e79375e into gridap:master Sep 6, 2024
2 checks passed
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