From 794f89a08cc02e5105c9485d4d5820da758694d6 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 09:44:01 +0200 Subject: [PATCH 01/17] chore: minimal pre-commit hooks --- .pre-commit-config.yaml | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6d75ff71..4aa8cc4c 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,4 @@ -default_stages: [commit, push] +default_stages: [push] repos: - repo: https://github.com/pycqa/isort @@ -48,6 +48,7 @@ repos: - repo: https://github.com/pycqa/pylint rev: v2.15.2 + stages: [commit] hooks: - id: pylint types: [python] From f7b60e2cae5093710bc0c78bc87884cb3043515c Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 09:59:39 +0200 Subject: [PATCH 02/17] ci: integration test before push --- .pre-commit-config.yaml | 13 +++++++++++-- pyproject.toml | 5 +++++ tests/test_train_model.py | 11 +++++++++++ 3 files changed, 27 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 4aa8cc4c..62fc7438 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -45,10 +45,19 @@ repos: entry: check-yaml language: python types: [yaml] - + + - repo: local + hooks: + - id: pytest + name: Run integration tests before push + entry: .venv/bin/pytest -m integration_test + language: script + types: [python] + stages: [push] + - repo: https://github.com/pycqa/pylint rev: v2.15.2 - stages: [commit] + stages: [commit, push] hooks: - id: pylint types: [python] diff --git a/pyproject.toml b/pyproject.toml index a101dc5c..6ed32fd7 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -70,6 +70,11 @@ good-names = "df,p,f,d,e,n,k,i,v,y_,X,y" disable = "too-many-lines,line-too-long,missing-raises-doc,no-self-argument,unused-wildcard-import,wildcard-import,no-else-return,too-many-arguments,redefined-outer-name,c-extension-no-member,wrong-import-order,import-outside-toplevel" extension-pkg-allow-list = "wandb" +[tool.pytest.ini_options] +markers = [ + "integration_test: Tests run on push. Should be as minimal as possible", +] + [tool.isort] known_third_party = ["wandb"] diff --git a/tests/test_train_model.py b/tests/test_train_model.py index f30cc85f..030f1d3d 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -21,3 +21,14 @@ def test_main(model_name): overrides=[f"+model={model_name}"], ) main(cfg) + + +@pytest.mark.integration_test +def test_integration_test(): + """test main using a variety of model.""" + with initialize(version_base=None, config_path="../src/psycopt2d/config/"): + cfg = compose( + config_name="integration_testing.yaml", + overrides=["+model=logistic-regression"], + ) + main(cfg) From 2901d5630d5750f471f2df026e3f3abb85b798e6 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:08:43 +0200 Subject: [PATCH 03/17] ci: check that tests are run on push --- .pre-commit-config.yaml | 6 +++--- tests/test_train_model.py | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 62fc7438..77dc2d03 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -54,17 +54,17 @@ repos: language: script types: [python] stages: [push] + always_run: true - repo: https://github.com/pycqa/pylint rev: v2.15.2 - stages: [commit, push] hooks: - id: pylint - types: [python] args: [ "-rn", # Only display messages "-sn", # Don't display the score "--disable=R,import-error" # Refactors are not important enough to block a commit. # Unused-imports aren't testable by the github action, so don't test that here. - ] \ No newline at end of file + ] + stages: [commit, push] \ No newline at end of file diff --git a/tests/test_train_model.py b/tests/test_train_model.py index 030f1d3d..5e07a318 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -31,4 +31,7 @@ def test_integration_test(): config_name="integration_testing.yaml", overrides=["+model=logistic-regression"], ) + + assert 2 == 1 + main(cfg) From a446418f2578efe10cdf1429d671990ad7c6cf88 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:26:52 +0200 Subject: [PATCH 04/17] fix: run on all files on push and autoinstall push --- .pre-commit-config.yaml | 39 ++++++++++++++++++++++++++------------- tests/test_train_model.py | 2 -- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 77dc2d03..7d5f6275 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,4 +1,5 @@ default_stages: [push] +default_install_hook_types: [pre-push, pre-commit] repos: - repo: https://github.com/pycqa/isort @@ -6,35 +7,43 @@ repos: hooks: - id: isort name: isort (python) - args: ["--profile", "black", "--filter-files", "--skip __init__.py"] + args: [".", "--profile", "black"] + always_run: true - repo: https://github.com/asottile/add-trailing-comma rev: v2.2.3 hooks: - id: add-trailing-comma + args: [--py36-plus] + always_run: true - repo: https://github.com/asottile/pyupgrade rev: v3.0.0 hooks: - id: pyupgrade args: ["--py39-plus"] + always_run: true - repo: https://github.com/myint/docformatter rev: v1.5.0 hooks: - id: docformatter - args: [--in-place] + args: [., --in-place, --recursive] + always_run: true - repo: https://github.com/psf/black rev: 22.6.0 hooks: - id: black + args: [src, tests] + always_run: true - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 hooks: - id: flake8 - args: [--config, .flake8] + args: [src, tests, --config, .flake8] + always_run: true - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.3.0 # Use the ref you want to point at @@ -45,15 +54,6 @@ repos: entry: check-yaml language: python types: [yaml] - - - repo: local - hooks: - - id: pytest - name: Run integration tests before push - entry: .venv/bin/pytest -m integration_test - language: script - types: [python] - stages: [push] always_run: true - repo: https://github.com/pycqa/pylint @@ -62,9 +62,22 @@ repos: - id: pylint args: [ + "src", + "tests", "-rn", # Only display messages "-sn", # Don't display the score "--disable=R,import-error" # Refactors are not important enough to block a commit. # Unused-imports aren't testable by the github action, so don't test that here. ] - stages: [commit, push] \ No newline at end of file + stages: [commit, push] + always_run: true + + - repo: local + hooks: + - id: pytest + name: Run integration tests before push + entry: .venv/bin/pytest -m integration_test + language: script + types: [python] + stages: [push] + always_run: true \ No newline at end of file diff --git a/tests/test_train_model.py b/tests/test_train_model.py index 5e07a318..cbc06fda 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -32,6 +32,4 @@ def test_integration_test(): overrides=["+model=logistic-regression"], ) - assert 2 == 1 - main(cfg) From d88f2296d05f67418ff72b21abd7fbd011bbc7cc Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:29:49 +0200 Subject: [PATCH 05/17] ci: remove unnecessary arguments Files are passed on git push, but not on pre-commit run --hook-types push :-( --- .pre-commit-config.yaml | 16 +++------------- tests/test_train_model.py | 2 ++ 2 files changed, 5 insertions(+), 13 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 7d5f6275..d984334e 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -7,43 +7,35 @@ repos: hooks: - id: isort name: isort (python) - args: [".", "--profile", "black"] - always_run: true + args: ["--profile", "black"] - repo: https://github.com/asottile/add-trailing-comma rev: v2.2.3 hooks: - id: add-trailing-comma - args: [--py36-plus] - always_run: true - repo: https://github.com/asottile/pyupgrade rev: v3.0.0 hooks: - id: pyupgrade args: ["--py39-plus"] - always_run: true - - repo: https://github.com/myint/docformatter rev: v1.5.0 hooks: - id: docformatter - args: [., --in-place, --recursive] - always_run: true + args: [--in-place, --recursive] - repo: https://github.com/psf/black rev: 22.6.0 hooks: - id: black args: [src, tests] - always_run: true - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 hooks: - id: flake8 - args: [src, tests, --config, .flake8] - always_run: true + args: [--config, .flake8] - repo: https://github.com/pre-commit/pre-commit-hooks rev: v4.3.0 # Use the ref you want to point at @@ -54,7 +46,6 @@ repos: entry: check-yaml language: python types: [yaml] - always_run: true - repo: https://github.com/pycqa/pylint rev: v2.15.2 @@ -70,7 +61,6 @@ repos: # Unused-imports aren't testable by the github action, so don't test that here. ] stages: [commit, push] - always_run: true - repo: local hooks: diff --git a/tests/test_train_model.py b/tests/test_train_model.py index cbc06fda..43ca268a 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -32,4 +32,6 @@ def test_integration_test(): overrides=["+model=logistic-regression"], ) + assert 2 == 3 + main(cfg) From 5603007e0e5d2089dceb4c4bdb0b020f068aa4dd Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:30:14 +0200 Subject: [PATCH 06/17] fix: remove dummy assert --- tests/test_train_model.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/tests/test_train_model.py b/tests/test_train_model.py index 43ca268a..cbc06fda 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -32,6 +32,4 @@ def test_integration_test(): overrides=["+model=logistic-regression"], ) - assert 2 == 3 - main(cfg) From 7c41a60644970672f77a9651bf345852775685c0 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:30:38 +0200 Subject: [PATCH 07/17] test: which hooks run From 14a802b6c65c7b2ae3e451941bfb0a6d9a17b75b Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:42:09 +0200 Subject: [PATCH 08/17] chore: remove unused about.py --- src/psycopt2d/about.py | 6 ------ 1 file changed, 6 deletions(-) delete mode 100644 src/psycopt2d/about.py diff --git a/src/psycopt2d/about.py b/src/psycopt2d/about.py deleted file mode 100644 index 99a76d8f..00000000 --- a/src/psycopt2d/about.py +++ /dev/null @@ -1,6 +0,0 @@ -# pylint: disable-all -import pkg_resources - -__version__ = pkg_resources.get_distribution("psycop-t2d").version -__title__ = "psycopt2d" -__download_url__ = "https://github.com/Aarhus-Psychiatry-Research/psycop-t2d.git" From 3d3b29d2624685fb32060657f4fde62130838276 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:42:21 +0200 Subject: [PATCH 09/17] ci: run push hooks on github actions as well --- .github/workflows/pre-commit.yml | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index cecce73a..e17a1033 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -15,4 +15,6 @@ jobs: - uses: actions/setup-python@v4 with: python-version: 3.9 - - uses: pre-commit/action@v3.0.0 \ No newline at end of file + - uses: pre-commit/action@v3.0.0 + with: + extra_args: "--hook-stage push --all-files" \ No newline at end of file From fc33ee8f29d0c0159e488b6e15a6bd930b87e54c Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:46:25 +0200 Subject: [PATCH 10/17] ci: only run pytest when relevant files have changed --- .pre-commit-config.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index d984334e..0dd93d9a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -51,6 +51,7 @@ repos: rev: v2.15.2 hooks: - id: pylint + types: [python] args: [ "src", @@ -65,9 +66,9 @@ repos: - repo: local hooks: - id: pytest + types: [python, yml, parquet, csv] name: Run integration tests before push entry: .venv/bin/pytest -m integration_test language: script types: [python] - stages: [push] - always_run: true \ No newline at end of file + stages: [push] \ No newline at end of file From bb3d484e4582f42e6b3d48ac39079c4572d2b0f9 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:57:21 +0200 Subject: [PATCH 11/17] ci: disable pytest pre-commit on GA using manual MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The manual stage is now for github actions. Allows disabling of the pytest pre-commit hook – it'll run as part of the testing action anyway. --- .github/workflows/pre-commit.yml | 2 +- .pre-commit-config.yaml | 10 ++++------ 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/.github/workflows/pre-commit.yml b/.github/workflows/pre-commit.yml index e17a1033..e0c1fe2a 100644 --- a/.github/workflows/pre-commit.yml +++ b/.github/workflows/pre-commit.yml @@ -17,4 +17,4 @@ jobs: python-version: 3.9 - uses: pre-commit/action@v3.0.0 with: - extra_args: "--hook-stage push --all-files" \ No newline at end of file + extra_args: "--hook-stage manual --all-files" \ No newline at end of file diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 0dd93d9a..171377f1 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -1,5 +1,5 @@ -default_stages: [push] -default_install_hook_types: [pre-push, pre-commit] +default_stages: [push, manual] +default_install_hook_types: [pre-push, pre-commit] # Using the "manual" stage for github actions repos: - repo: https://github.com/pycqa/isort @@ -59,16 +59,14 @@ repos: "-rn", # Only display messages "-sn", # Don't display the score "--disable=R,import-error" # Refactors are not important enough to block a commit. - # Unused-imports aren't testable by the github action, so don't test that here. + # Unused-imports aren't testable by the github action without installing the whole project, so don't test that here. ] - stages: [commit, push] - repo: local hooks: - id: pytest - types: [python, yml, parquet, csv] + types: [python, yaml, csv] name: Run integration tests before push entry: .venv/bin/pytest -m integration_test language: script - types: [python] stages: [push] \ No newline at end of file From a10cde06de23aaf62cfda7163825524211f24938 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:59:10 +0200 Subject: [PATCH 12/17] ci: remove unneeded recursive flag from docformatter Files are passed to it on push anyways. --- .pre-commit-config.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 171377f1..25603274 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -23,7 +23,7 @@ repos: rev: v1.5.0 hooks: - id: docformatter - args: [--in-place, --recursive] + args: [--in-place] - repo: https://github.com/psf/black rev: 22.6.0 From f2d92f130975999b394d63f65a893135d55ad3d6 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 10:59:38 +0200 Subject: [PATCH 13/17] ci: remove unnecessary black args --- .pre-commit-config.yaml | 1 - 1 file changed, 1 deletion(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 25603274..2760c590 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,7 +29,6 @@ repos: rev: 22.6.0 hooks: - id: black - args: [src, tests] - repo: https://github.com/PyCQA/flake8 rev: 5.0.4 From dad56827645651137d7a3034ba880db7e9f42481 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 11:01:59 +0200 Subject: [PATCH 14/17] ci: lint only changed files on push --- .pre-commit-config.yaml | 2 -- 1 file changed, 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 2760c590..bc1ceb6a 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -53,8 +53,6 @@ repos: types: [python] args: [ - "src", - "tests", "-rn", # Only display messages "-sn", # Don't display the score "--disable=R,import-error" # Refactors are not important enough to block a commit. From 509f4cbadf74e68ab49394e386c4c2b780b9febe Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Sat, 8 Oct 2022 11:04:09 +0200 Subject: [PATCH 15/17] ci: fix integration_test had false negatives Remove types --- .pre-commit-config.yaml | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index bc1ceb6a..2d5e4ba5 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -62,8 +62,9 @@ repos: - repo: local hooks: - id: pytest - types: [python, yaml, csv] name: Run integration tests before push entry: .venv/bin/pytest -m integration_test language: script - stages: [push] \ No newline at end of file + stages: [push] + pass_filenames: false + always_run: true \ No newline at end of file From 0e30f4007960bf17f762265c6e167daae2869e08 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Mon, 10 Oct 2022 12:30:59 +0200 Subject: [PATCH 16/17] fix: rename integration tests --- .pre-commit-config.yaml | 2 +- pyproject.toml | 2 +- tests/test_train_model.py | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 1fa1c66d..093c3020 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -75,7 +75,7 @@ repos: hooks: - id: pytest name: Run integration tests before push - entry: .venv/bin/pytest -m integration_test + entry: .venv/bin/pytest -m pre_push_test language: script stages: [push] pass_filenames: false diff --git a/pyproject.toml b/pyproject.toml index 9a7f47b5..3c66bd91 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -73,7 +73,7 @@ extension-pkg-allow-list = "wandb" [tool.pytest.ini_options] markers = [ - "integration_test: Tests run on push. Should be as minimal as possible", + "pre_push_test: Tests run on push. Should be as minimal as possible to maintain fast push speeds.", ] [tool.isort] diff --git a/tests/test_train_model.py b/tests/test_train_model.py index 40533a2d..b82ab4eb 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -18,9 +18,9 @@ def test_main(model_name): main(cfg) -@pytest.mark.integration_test +@pytest.mark.pre_push_test def test_integration_test(): - """test main using a variety of model.""" + """test main using the logistic model. Used for quickly testing functions before a push.""" with initialize(version_base=None, config_path="../src/psycopt2d/config/"): cfg = compose( config_name="integration_testing.yaml", @@ -28,6 +28,7 @@ def test_integration_test(): ) main(cfg) + def test_crossvalidation(): """Test crossvalidation.""" with initialize(version_base=None, config_path="../src/psycopt2d/config/"): From e2528cbd3dfe8f67a67950cbcf7f13c39b7dbbf6 Mon Sep 17 00:00:00 2001 From: Martin Bernstorff Date: Mon, 10 Oct 2022 12:31:35 +0200 Subject: [PATCH 17/17] lint --- tests/test_train_model.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests/test_train_model.py b/tests/test_train_model.py index b82ab4eb..76217236 100644 --- a/tests/test_train_model.py +++ b/tests/test_train_model.py @@ -20,7 +20,10 @@ def test_main(model_name): @pytest.mark.pre_push_test def test_integration_test(): - """test main using the logistic model. Used for quickly testing functions before a push.""" + """test main using the logistic model. + + Used for quickly testing functions before a push. + """ with initialize(version_base=None, config_path="../src/psycopt2d/config/"): cfg = compose( config_name="integration_testing.yaml",