Skip to content

First draft commit of GridVisualize-Doc with Examplejuggler #67

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

Merged
merged 1 commit into from
Mar 12, 2025

Conversation

Liameloy
Copy link
Member

@Liameloy Liameloy commented Mar 10, 2025

Request to review the documentation created with ExampleJuggler.jl integrated into the process.
Multiple files were no longer necessary, therefore have been removed (though still existing on my machine in a separate folder, should they be necessary).
I mostly used the make.jl of SimplexGridFactory.jl as a kind of template and then added/changed things to make the documentation buildable and make it look as close to the original documentation as possible.

Following things have been done to achieve this and with the goal of reducing "file-clutter" (e.g. needing to add the same functions to multiple files) in mind:
Changed make.jl to use the macros @docscripts/@docplutonotebooks
Added a generateplots-function to plotting.jl since @docscripts-macro requires this, effectively combining plotting.jl and makeplots.jl and using the contents of makeplots.jl for generateplots.
Due to this makeplots.jl and makefigs.jl were no longer necessary, so i removed them from my local GridVisualize-Project.
In the first run of trying to build this doc, multiple times errors like "Package xyz missing, run 'using Pkg; add Pkg("xyz")'" turned up, so I added them.

@Liameloy Liameloy marked this pull request as draft March 10, 2025 09:12
@pjaap pjaap self-requested a review March 10, 2025 09:14
@Liameloy
Copy link
Member Author

I can already see that the code quality check has failed, I may have forgot to run runic before pushing.

@pjaap
Copy link
Member

pjaap commented Mar 10, 2025

Yes, please make sure that code quality passes. However, the failing unit tests are caused by a (probably unrelated) Python error in the runner. We had this before in another PR in this repo. I'll take a closer look there

@Liameloy
Copy link
Member Author

Reviewing the code quality check result yields clarity. I misspelled "successfully".

@pjaap
Copy link
Member

pjaap commented Mar 10, 2025

Please use pre-commit in your machine to get automatic fixes of the code quality issues.

@Liameloy Liameloy force-pushed the dev/docsViaExampleJuggler branch from 3bad6be to 2b3daf1 Compare March 10, 2025 15:23
@pjaap
Copy link
Member

pjaap commented Mar 10, 2025

Great. I think you get meaningful error messages now form the CI. There are some files included that do not exist any more.

Copy link
Member

@pjaap pjaap left a comment

Choose a reason for hiding this comment

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

Thanks for the rework.

However, some parts are not clear to me, which is not that bad.

Please write a small recap (list) of what you did in the first comment of this PR (edit the comment). Then we can approve the changes.

@j-fu
Copy link
Member

j-fu commented Mar 10, 2025

Hi, thanks for the initiative, and for the work so far !

  • I wonder, why the new additions in Project.toml are needed.
  • Also I suggest to keep pyplot out of the game for the docs (besides testing) and to focus on CairoMakie - just in order to prevent annoying hiccups with the python installation.

@Liameloy
Copy link
Member Author

In regards to the added Packages:
PyPlot has been removed. As I used the make.jl from SimplexGridFactory.jl, which has plots in PyPlot as well, as a pseudo-template, I initially added PyPlot to see what would work.

PlutoSliderServer is necessary for the @docplutonotebook-macro. If it is not imported in the make.jl it yields this error:
'LoadError: Please import/use PlutoSliderServer.jl in order to use docplutonotebooks with iframe=true'

The adding of ExampleJuggler is selfexplanatory i reckon.

@j-fu
Copy link
Member

j-fu commented Mar 11, 2025

Yes, but they are needed just in docs/Project.toml, but not in the Project.toml of the package.
When running documentation generation, one just needs to work in the docs environment, not in the package environment, like

julia --project=docs docs/make.toml

I tried to write about environments, package workflow etc. here. Please let me know if something is missing there.

@Liameloy
Copy link
Member Author

Ah, that clears things up. I was aware that the docs was a project on its own, but I did not know that the "parent"-Package of the docs-Package did not need to include the same packages.

I was thinking in ways of some inheritance structure for some reason.

It will be (has been) adjusted and will be included in the next push.

@Liameloy Liameloy force-pushed the dev/docsViaExampleJuggler branch from 8fed963 to c62ab61 Compare March 12, 2025 09:57
@Liameloy
Copy link
Member Author

squashed and ready

@pjaap pjaap marked this pull request as ready for review March 12, 2025 10:40
@pjaap pjaap merged commit b6dae96 into WIAS-PDELib:main Mar 12, 2025
9 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.

3 participants