Skip to content
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

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 #30

Merged
merged 36 commits into from
Nov 28, 2024

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
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