pySTEPS / pysteps

Python framework for short-term ensemble prediction systems.
https://pysteps.github.io/
BSD 3-Clause "New" or "Revised" License
466 stars 168 forks source link

[WIP] Decouple examples gallery from the main repository #251

Closed aperezhortal closed 2 years ago

aperezhortal commented 2 years ago

Problem

Currently, the examples gallery is a part of the pysteps project and the examples are executed during the documentation build in Read The Docs (RTD). Running these examples is a time-consuming operation that puts us close to the RTD's build limits.

Proposed solution

The discussions in #242 converge on decoupling the example gallery from the main repo, keeping the examples in a separate repository that uses Github Actions to execute the examples and build the examples gallery. The rendered gallery is pulled from the separate repo when the pysteps documentation is built in RTD.

Changes introduced in this PR

Should we use Jupyter notebooks or python scripts for the example gallery?

I personally prefer python scripts. Although jupyter notebooks are good for exploratory analysis and can be nicely visualized in Github, keeping track of the changes is very difficult since the diffs are not very human-readable. On the other hand, keeping track of the differences in python scripts is straightforward. Another advantage of using python scripts is that we can benefit from the static code analysis of the IDEs.

Also, sphinx-gallery automatically converts the python scripts to jupyter notebooks and makes them available in the documentation. There is an option (experimental) to add the open in binder button that we may explore.

I check how the example galleries are handled in other projects and some use jupyter notebooks while others use scripts:

It seems that it boils down to a matter of personal preference. Both approaches are good, and if we choose to use notebooks, updating the current PR is straightforward.

What are your thoughts?

codecov[bot] commented 2 years ago

Codecov Report

Merging #251 (baedcdd) into master (bd94785) will not change coverage. The diff coverage is n/a.

:exclamation: Current head baedcdd differs from pull request most recent head c4e4aef. Consider uploading reports for the commit c4e4aef to get more accurate results Impacted file tree graph

@@           Coverage Diff           @@
##           master     #251   +/-   ##
=======================================
  Coverage   80.78%   80.78%           
=======================================
  Files         140      140           
  Lines       10625    10625           
=======================================
  Hits         8583     8583           
  Misses       2042     2042           
Flag Coverage Δ
unit_tests 80.78% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.


Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update bd94785...c4e4aef. Read the comment docs.

dnerini commented 2 years ago

I also noticed that pysteps_tutorials uses dev as default branch. Is this intentional?

aperezhortal commented 2 years ago

I also noticed that pysteps_tutorials uses dev as default branch. Is this intentional?

Just a consequence of the branch that I was using :smile: I pushed the dev commits to the main branch, and set the latter as default.

dnerini commented 2 years ago

I have another annoying comment (sorry): it should be "Example gallery" all over instead of "Examples gallery", shouldn't it?

aperezhortal commented 2 years ago

... it should be "Example gallery" all over instead of "Examples gallery"

Yes, you are right. I will fix it shortly.

I will also change the PR to Work In progress since I'm experimenting with a slightly different workflow where the rendering is triggered by empty commit messages to a dedicated branch. This allows finer control on which pysteps branch to pull, which branch in the gallery to render, and what RTD branch to trigger. :nerd_face:

aperezhortal commented 2 years ago

PR #262 implements a different workflow that I think will be easier to maintain since the example gallery is not decoupled.

Do we keep this PR open? Or do we close it in favor of #262?

dnerini commented 2 years ago

Yes, I agree, let's close this PR in favor of #262.