search

open-atmos / devops_tests

pytest test routines asserting for GitHub issue-linked TODO labelling in the code, README link consistency and some Jupyter notebook sanity checks
GNU General Public License v3.0
0 stars 4 forks source link

add pre-commit hook to enforce notebooks run order #36

Closed AgnieszkaZaba closed 1 month ago

AgnieszkaZaba commented 1 month ago

Solution to #27 as pre-commit hook

AgnieszkaZaba commented 1 month ago

For resolving #16 we need to add another hook to check if at least one output isn't an empty list.

slayoo commented 1 month ago

BTW, this PR contains 27 commits as of now, most of which are unrelated. It is because the branch was branched off a previous PR's branch, instead of from main branch. It can be resolved by rebasing this PR's branch to the current main.

slayoo commented 1 month ago

@AgnieszkaZaba, this PR is of course OK, but it actually does not help us with maintenance of PySDM, PyPartMC or PyMPDATA - the pre-commit hook would need to be added in these repos, not here. I wonder if creating a pytest test to check for cell execution order wouldn't still be worth adding?

AgnieszkaZaba commented 1 month ago

@AgnieszkaZaba, this PR is of course OK, but it actually does not help us with maintenance of PySDM, PyPartMC or PyMPDATA - the pre-commit hook would need to be added in these repos, not here. I wonder if creating a pytest test to check for cell execution order wouldn't still be worth adding?

Probably it would be better and easier to add check for output.

AgnieszkaZaba commented 1 month ago

BTW, this PR contains 27 commits as of now, most of which are unrelated. It is because the branch was branched off a previous PR's branch, instead of from main branch. It can be resolved by rebasing this PR's branch to the current main.

@slayoo I think I did something wrong here

slayoo commented 1 month ago

BTW, this PR contains 27 commits as of now, most of which are unrelated. It is because the branch was branched off a previous PR's branch, instead of from main branch. It can be resolved by rebasing this PR's branch to the current main.

@slayoo I think I did something wrong here

reverted. let's merge and keep the issue open Thanks!

AgnieszkaZaba commented 1 month ago

BTW, this PR contains 27 commits as of now, most of which are unrelated. It is because the branch was branched off a previous PR's branch, instead of from main branch. It can be resolved by rebasing this PR's branch to the current main.

@slayoo I think I did something wrong here

reverted. let's merge and keep the issue open Thanks!

Thanks!