-
Notifications
You must be signed in to change notification settings - Fork 29
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 to analysis doc #504
add to analysis doc #504
Conversation
Some links in the |
@miniufo This is fantastic, thank you so much. Looks great, thank you for this contribution! I just went through it and I basically only changed, apart from small things,
If the documentation successfully builds then you can check what it looks like here https://speedyweather.github.io/SpeedyWeather.jl/previews/PR504 so that you can approve before it gets merged! Can you actually see the details of why the documentation is failing? (Maybe not because you're not part of our organisation, yet?) |
One error was also thrown because you wrote |
Totally agreed with all your points. It is great that everything is automatic in the flow. But there is a two-language problem for me: I use python, so writing julia codes for a nice plot (panels, share axes etc.) is not easy for me. So the plot is generated by matplotlib which I used a lot. I just learn a lot from your suggestions (julia levels up...).
I cannot see anything from the link (404 file not found). The checks are not completed (the third one) but I can't find any buttom to click: |
That's the same for me right now, normally you can just see a preview of the new docs but for some reason it doesn't deploy. I'm on it and ping you when it is! |
On plotting, yeah at some point I want to do it all with Makie but I'm also still way more used to matplotlib. Hence it's PythonPlot in the documentation for now and that's fine. |
Apparently it doesn't deploy because
@navidcy We can't merge because it isn't deploying, and it's not deploying because the PR comes from a fork. Any idea how to solve this? Shall we first merge it into a branch in this repo and then into main? |
we can merge I think -- it'll deploy then. or we can invite @miniufo as a collaborator in the repo and do what @josuemtzmo did (which I don't know really) to transform all the commits to this PR -> CliMA/Oceananigans.jl#3506 that was coming from their fork to this PR -> CliMA/Oceananigans.jl#3512 on Oceananigans repo. |
closing in favor of #505 |
I've add some contents to the analysis doc. In the last section I put all of the diagnostics into a callback, following @milankl's tips. Please check if everything looks OK.