Skip to content

Conversation

@ovanvincq
Copy link
Contributor

Hi,

This PR is a proposal to transform GridapMakie.jl into an extension of Gridap.jl.
I named this extension GridapMakieExt.

It is compatible with Makie.jl v0.24 and not with previous versions because the recipes use the new ComputeGraph interface.

@ovanvincq
Copy link
Contributor Author

Tests fail because:

  • Makie requires at least julia v1.10
  • ci.yml should be modified to activate OpenGL capabilities

@JordiManyer
Copy link
Member

Hi @ovanvincq thank you for the work.

I do have some preliminary comments:

  • I think it is extremely important we acknowledge the original creators of the package. I would add a clear admonition block at the top of the documentation page for the extension referencing the original package, the original authors and the fact that is was created during a Google SummerOfCode.
  • I would put the images into a subfolder of assets.
  • In terms of organizing the documentation, I would add a separator for extensions, and list the file as Makie extension instead. There will be some major documentation overhaul soon-ish, so it will probably be moved around again, but I think that should be ok for now.
  • Removing compatibility for 1.9 is OK, I think.
  • I see what you mean by the OpenGL stuff. In light of this, I would split the tests into their own CI job, and just copy-paste the CI file we have in the package. I think I have some ideas on how to unify all of this, but splitting like this should be ok for now.

@ovanvincq
Copy link
Contributor Author

ovanvincq commented Jun 30, 2025

Hi @JordiManyer

I made the changes in the documentation.

For OpenGL, I think you need to change run: julia... to run: DISPLAY=:0 xvfb-run -s '-screen 0 1024x768x24' julia... in ci.yml
If you prefer, I can also include CairoMakie in the weak dependencies and run the test with CairoMakie

A quick question: does a Triangulation{0,N} with N>1 exist?

@JordiManyer
Copy link
Member

As I said, we can split the tests and run the ones we already had in GridapMakie.jl, I'll unify them again in a later PR. Basically copy paste the GridapMakie CI. You could also do the CairoMakie workaround, of it works.

A quick question: does a Triangulation{0,N} with N>1 exist?

Yes, a triangulatin of points would be Triangulation{0,D} with D the ambient dimension. I don't know if anyone would ever want to use it, but that is what it would be. I would say it's not super important to support that on Makie.

@codecov
Copy link

codecov bot commented Jun 30, 2025

Codecov Report

Attention: Patch coverage is 87.12121% with 34 lines in your changes missing coverage. Please review.

Project coverage is 87.27%. Comparing base (c2a8af2) to head (8c7298b).

Files with missing lines Patch % Lines
ext/GridapMakieExt/recipes.jl 80.70% 22 Missing ⚠️
ext/GridapMakieExt/conversions.jl 91.94% 12 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1121      +/-   ##
==========================================
- Coverage   89.23%   87.27%   -1.97%     
==========================================
  Files         208      211       +3     
  Lines       26196    26457     +261     
==========================================
- Hits        23377    23091     -286     
- Misses       2819     3366     +547     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ovanvincq
Copy link
Contributor Author

@JordiManyer I copied the CI file from GridapMakie and the test succeeded

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