[INFRA] Python test automation #44

Remi-Gau · 2022-04-07T11:46:57Z

Because this has not been merged, the results of the Github continuous integration woklfows are for now only viewable in the action tab of my fork

The initial commit of this PR is common to #44 (some changes are common to both PR and this might help prevent merge conflicts).

relates to #33

TODO

investigate test failure with python 3.10 (see here)
set up code coverage upload on codecov

DONE

Common base for the matlab and python testing branch to minimize merge conflicts down the line

.github/workflows/run_demos_python.yml

Remi-Gau · 2022-04-07T12:25:47Z

.github/workflows/run_tests_python.yml

+    runs-on: ${{ matrix.os }}
+    strategy:
+      matrix:
+        os: [ubuntu-20.04]


Do we want to test with different OS?

probably higher priority is different python versions (since python versions seem to cause more problems)... I suppose, assuming that the tests can run pretty quickly.

Now set up to test in parallel python 3.7 to python 3.9 on ubuntu, mac, windows

Running the tests themselves takes about a minute in CI. But the whole takes a bit more time than this because of the set up and tear down

tests/test_glmsingleunit.py

Remi-Gau · 2022-04-07T13:02:16Z

Python demos run in 35-40 minutes in CI (see here).

So running it on every commit of every pull-request might be a bit much.

Maybe a better approach could be to run it:

only on push to the master branch (so only once when the PR is merged)
at regular intervals with a CRON job (see here)

Remi-Gau · 2022-04-12T12:22:20Z

@iancharest

Trying to test against actual values, I realizing that the HRFindex in the output flles of the different models have different shapes.

Is this intentional?

In [65]: tmp = np.load("TYPEB_FITHRF.npy", allow_pickle=True)
    ...: print(tmp.item(0)["HRFindex"].shape)
    ...: 
    ...: tmp = np.load("TYPEC_FITHRF_GLMDENOISE.npy", allow_pickle=True)
    ...: print(tmp.item(0)["HRFindex"].shape)
    ...: 
    ...: tmp = np.load("TYPED_FITHRF_GLMDENOISE_RR.npy", allow_pickle=True)
    ...: print(tmp.item(0)["HRFindex"].shape)
(20, 20, 1)
(20, 20, 1)
(400,)

Remi-Gau · 2022-04-12T15:08:06Z

@iancharest

Trying to test against actual values, I realizing that the HRFindex in the output flles of the different models have different shapes.

Looks like this is because there is not a

GLMsingle/glmsingle/glmsingle.py

Line 1386 in 0d835eb

'HRFindex': HRFindex.reshape(xyz),

in an "else" block for model 3 as there is for model 2.

Remi-Gau · 2022-04-12T15:13:59Z

Maybe a better approach could be to run it:

1. only on `push` to the master branch (so only once when the PR is merged)

2. at regular intervals with a CRON job (see [here](https://jasonet.co/posts/scheduled-actions/))

This is now set up

Remi-Gau · 2022-04-12T15:20:23Z

README.md

+
+[![Python tests](https://github.com/cvnlab/GLMsingle/actions/workflows/run_tests_python.yml/badge.svg)](https://github.com/cvnlab/GLMsingle/actions/workflows/run_tests_python.yml)
+[![Python demos](https://github.com/cvnlab/GLMsingle/actions/workflows/run_demos_python.yml/badge.svg)](https://github.com/cvnlab/GLMsingle/actions/workflows/run_demos_python.yml)
+[![Python-coverage](https://codecov.io/gh/Remi-Gau/GLMsingle/branch/main/graph/badge.svg?token=H75TAAUVSW)](https://codecov.io/gh/Remi-Gau/GLMsingle)


The code coverage is not yet uploaded and so this might require some tweaking after merging:

setting up an account for cvnlab on codecov (can simply link the github account)

changing the URL of this badge

But once set up you can know how pull request change the code coverage: like this
Remi-Gau#3 (comment)

Remi-Gau · 2022-04-12T15:27:17Z

README.md


 Notes:
-* Please note that GLMsingle is not (yet) compatible with Python 3.9 (due to an incompatibility between scikit-learn and Python 3.9). Please use Python 3.8 or earlier.


I have removed this since the tests pass with python 3.9.

But there seem to be a different issue with python 3.10 and numpy: I can investigate but I would not consider necessary for this PR

Remi-Gau · 2022-04-12T15:30:34Z

requirements.txt

+fracridge>=1.4.3
+h5py>=3.1.0
+matplotlib>=3.3.4
+nibabel>=3.2.2
+numpy>=1.17.0
+scikit-learn>=0.23.2
+scipy>=1.5.4
+tqdm>=4.63.1


I have pinned the dependency minimum version for GLMsingle to be those that were installed by default when using python 3.6.

Remi-Gau · 2022-04-12T15:31:14Z

setup.py

-    version='0.0.1',
+    version='1.0.0',


To match what the README and the github tag say

Remi-Gau · 2022-04-12T15:31:52Z

setup.py

@@ -28,5 +28,6 @@
    packages=find_packages(),
    include_package_data=True,
    zip_safe=False,
-    install_requires=requires
+    install_requires=requires,
+    python_requires='>=3.6, <3.10'


Specifying which python version to use

Remi-Gau · 2022-04-12T15:34:15Z

tests/test_GLM_single.py

+# TODO use same expected data for Python and MATLAB ?
+# TODO in both cases expected results should probably be in a set of CSV files
+# tmp = sio.loadmat(join(expected_dir, "TYPEB_FITHRF.mat"))
+# expected["typeb"]["HRFindex"] = tmp["HRFindex"]


In the long run we probably do not want to have different expected data for MATLAB and python, a set of CSV files to read from for both languages could be easier to handle, more interoperable and easier to inspect too.

Remi-Gau · 2022-04-12T15:41:37Z

@iancharest
I am pretty much done with this so feel free to have a look when you can.

Remi-Gau · 2022-04-12T15:49:42Z

CONTRIBUTING.md

+[black](https://black.readthedocs.io/en/stable/) (code formater) and
+[flake8](https://flake8.pycqa.org/en/latest/) (style guide enforcement) are used
+on the test code base.
+
+Ypu can use make to run them automatically with
+
+```bash
+make lint/black # to run black
+make lint/flake8 # to run flake8
+make lint # to run both
+```


Some of those are for code styling, so it does not have to be done manually. Only applies to the test code base.

Can remove it if you want.

kendrickkay · 2022-04-30T21:45:49Z

@iancharest
Trying to test against actual values, I realizing that the HRFindex in the output flles of the different models have different shapes.

Looks like this is because there is not a

GLMsingle/glmsingle/glmsingle.py

Line 1386 in 0d835eb

'HRFindex': HRFindex.reshape(xyz),

in an "else" block for model 3 as there is for model 2.

I think that sounds about right. Basically, the code tries to flexibly handle X Y Z (volume) data, or A (surface) data, and there are funny reshapings that have to happen. Hopefully you and @iancharest can sort this out and put this into this pull request.

Remi-Gau added 6 commits April 7, 2022 13:12

Add common tools for MATLAB and Python testing

af934b4

Common base for the matlab and python testing branch to minimize merge conflicts down the line

pin minimum python and dependencies versions

5dfcecd

add config for flake8 and pytest

ddbf5b2

add smoke test

8a7c1f5

automate python test with Github CI

53c1804

automate testing python demos with github CI

922c8d0

Remi-Gau marked this pull request as draft April 7, 2022 11:47

Remi-Gau commented Apr 7, 2022

View reviewed changes

.github/workflows/run_demos_python.yml Outdated Show resolved Hide resolved

Remi-Gau commented Apr 7, 2022

View reviewed changes

tests/test_glmsingleunit.py Outdated Show resolved Hide resolved

This was referenced Apr 7, 2022

[INFRA] MATLAB test automation #43

Open

[DOC] Add sphinx documentation and serve on read the docs #45

Merged

[WIP][ENH] add Dockerfile for python version og GLM single #46

Draft

Remi-Gau added 4 commits April 12, 2022 10:51

update python versions to use in CI

3ee979b

add base requirements for dev install

d251cee

add CRON job for demos in CI and fail tests ealy

5886906

only use pytest-cov for coverage

78e0727

Remi-Gau added 7 commits April 12, 2022 16:28

test GLM_single against expected data

d5ee9bf

run tests on all OS and update CI comment

f4aaecb

lint

e4cb204

add dev doc

e1aa83b

run demos on ubuntu latest in CI

f76552c

add badges

a27c0aa

test a forced failure of demos

66a5d19

Remi-Gau commented Apr 12, 2022

View reviewed changes

only run tests on push on main

0331ef5

Remi-Gau commented Apr 12, 2022

View reviewed changes

setup.py

version='0.0.1',

version='1.0.0',

Copy link

Contributor Author

Remi-Gau Apr 12, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

To match what the README and the github tag say

Remi-Gau commented Apr 12, 2022

View reviewed changes

revert forced demo failure

71c3a53

Remi-Gau changed the title ~~[WIP][INFRA] Python test automation~~ [INFRA] Python test automation Apr 12, 2022

Remi-Gau marked this pull request as ready for review April 12, 2022 15:37

only run demos in CI on schedule

14d83d9

Remi-Gau commented Apr 12, 2022

View reviewed changes

Remi-Gau added 2 commits April 13, 2022 09:02

add test on R2

a972d94

fix test

9daa42d

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[INFRA] Python test automation #44

[INFRA] Python test automation #44

Remi-Gau commented Apr 7, 2022 •

edited

Loading

Remi-Gau Apr 7, 2022

kendrickkay Apr 7, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau commented Apr 7, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau Apr 12, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau Apr 12, 2022

kendrickkay commented Apr 30, 2022


		Notes:
		* Please note that GLMsingle is not (yet) compatible with Python 3.9 (due to an incompatibility between scikit-learn and Python 3.9). Please use Python 3.8 or earlier.

[INFRA] Python test automation #44

Are you sure you want to change the base?

[INFRA] Python test automation #44

Conversation

Remi-Gau commented Apr 7, 2022 • edited Loading

TODO

DONE

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Remi-Gau commented Apr 7, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau commented Apr 12, 2022

Remi-Gau commented Apr 12, 2022

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Remi-Gau commented Apr 12, 2022

Choose a reason for hiding this comment

kendrickkay commented Apr 30, 2022

Remi-Gau commented Apr 7, 2022 •

edited

Loading