Skip to content

Conversation

@Sfonxu
Copy link
Contributor

@Sfonxu Sfonxu commented Nov 27, 2024

Pull request containing function as mentioned in #29

@slayoo slayoo changed the title add show_anim() to jupyter_utils add show_anim(); bump checkout & setup-python actions in pylint CI job; move more setup entries into pyproject.toml; add example notebook with animation Nov 27, 2024
@slayoo
Copy link
Member

slayoo commented Nov 27, 2024

@Sfonxu, thanks!

It would be great to:

  • add a brief description of the new routine to the README and
  • add a CI workflow that executes the test notebook (we can use the test_run_notebooks.py from the devops_tests repo)

BTW, please avoid making pull requests from your fork's main branch as:

  • it gives other write permission to your main branch,
  • it prevents having multiple PRs from one repo (unable to branch out of clean main).
  • will make your main branch in conflict with the upstream main after merging the PR (since we do squash-merges, i.e. you branch will be rendered at the same time behind and ahead of upstream:main)

@slayoo slayoo linked an issue Nov 28, 2024 that may be closed by this pull request
@slayoo
Copy link
Member

slayoo commented Nov 28, 2024

Also:

  • the demo notebook was committed with output cleaned up, thus not rendering the animation

@Sfonxu
Copy link
Contributor Author

Sfonxu commented Nov 28, 2024

@Sfonxu, thanks!

It would be great to:

* [ ]  add a brief description of the new routine to the README and

* [ ]  add a CI workflow that executes the test notebook (we can use the `test_run_notebooks.py` from the `devops_tests` repo)

BTW, please avoid making pull requests from your fork's main branch as:

* it gives other write permission to your `main` branch,

* it prevents having multiple PRs from one repo (unable to branch out of clean `main`).

* will make your `main` branch in conflict with the upstream `main` after merging the PR (since we do squash-merges, i.e. you branch will be rendered at the same time behind and ahead of `upstream:main`)

Thank you for the feedback! I didn't even realize I made the request from main branch. I have added a small description to README.md as well as fixed the notebook and made a test workflow for the whole thing. I'll be more careful next time and make sure I'm actually on a branch.

@slayoo slayoo changed the title add show_anim(); bump checkout & setup-python actions in pylint CI job; move more setup entries into pyproject.toml; add example notebook with animation create test and pylint CI workflows; add show_anim(); bump checkout & setup-python actions in pylint CI job; move more setup entries into pyproject.toml; add example notebook with animation Nov 28, 2024
@slayoo slayoo changed the title create test and pylint CI workflows; add show_anim(); bump checkout & setup-python actions in pylint CI job; move more setup entries into pyproject.toml; add example notebook with animation create test and pylint CI workflows; use devops_tests as a submodule; add show_anim(); bump checkout & setup-python actions in pylint CI job; move more setup entries into pyproject.toml; add example notebook with animation Nov 28, 2024
@slayoo
Copy link
Member

slayoo commented Nov 28, 2024

KUDOS @Sfonxu !

@slayoo slayoo added this pull request to the merge queue Nov 28, 2024
Merged via the queue into open-atmos:main with commit 653c2ba Nov 28, 2024
3 checks passed
@slayoo slayoo mentioned this pull request Nov 28, 2024
@slayoo slayoo mentioned this pull request Apr 6, 2025
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.

introduce show_anim() to automagicaly generate notebook-embedded gifs

2 participants