Pytests creator #8
Conversation
filter unit test
n out but num_edge_too_much goes to 0
Main merge
There was a problem hiding this comment.
Good job! Here are a few comments to start with.
- Make sure your code fulfills the pep8 requirements. You can pip install flake8 and run it on your code. There are a few lines that are too long and spaces missing. (come and see me for help)
- Find a way to remove hardcoded paths from the code. As I said in the comment, go see how we do in scilpy.
- I think we should follow the same structure as scilpy. Thus, instead of a
pytestdirectory, you could ad atestsdirectory in each module and put the tests there. Go see how we organize it in scilpy. It is very similar to what you did but structured a bit differently.
Don't hesitate if you have questions!
pytest/adj_matrix/test_filter.py
Outdated
| @pytest.mark.parametrize("graph, node_indices, expected", test_data) | ||
| def test_remove_orphan_nodes(graph, node_indices, expected): | ||
|
|
||
| out_graph,out_ind = remove_orphan_nodes(graph, node_indices) |
There was a problem hiding this comment.
Always put an empty space after a comma.
pytest/adj_matrix/test_filter.py
Outdated
|
|
||
| @pytest.mark.parametrize("graph, node_indices, expected", test_KEEP) | ||
| def test_remove_orphan_nodes_KEEP(graph, node_indices, expected): | ||
| out_graph,out_ind = remove_orphan_nodes(graph, node_indices, keep_indices=np.array([2])) |
There was a problem hiding this comment.
Same thing here for the comma.
pytest/adj_matrix/test_filter.py
Outdated
|
|
||
| out_graph = remove_intermediate_connections(graph) | ||
| assert np.array_equal(out_graph, expected) | ||
| assert np.array_equal(remove_intermediate_connections(graph,node_indices,keep_indices=[1]),graph) |
pytest/adj_matrix/test_reconst.py
Outdated
|
|
||
|
|
||
| test_data = [ | ||
| (nib.load('/home/kevin-da/LibrairiesQuack/Quackto/quactography/data/simplePhantoms/fanning_2d_3bundles/wm_vf.nii.gz'), |
There was a problem hiding this comment.
This hardcoded path will not work on someone else's computer. You can do something like we do in scilpy. For instance, the scilpy_bot uses a file in data. Go see scil_search_keywords.py and scilpy_bot.py from scilpy.utils.
|
|
||
|
|
||
| def test_quac_mat_adj_build(): | ||
| script_path = str_path + "/scripts/quac_matrix_adj_build.py" |
There was a problem hiding this comment.
We should follow the same method as Scilpy for testing the scripts. You can refer to any scilpy script test (here is an example: https://github.com/scilus/scilpy/blob/master/scripts/tests/test_aodf_metrics.py). If you pip install -e . the repo, you should have the scripts in your path and be able to use lines like ret = script_runner.run('scil_aodf_metrics.py', '--help').
|
|
||
| def test_quac_rndmat_adj_build(): | ||
| script_path = os.path.abspath( | ||
| "/home/kevin-da/LibrairiesQuack/Quackto/quactography/scripts/quac_randmatrix_adj_build.py" |
There was a problem hiding this comment.
You forgot a hardcoded path here and below.
| @@ -8,26 +8,20 @@ | |||
| str_path = str(DIR_PATH) | |||
There was a problem hiding this comment.
Remove this line and apply the str() directly in line 6.
| assert result.returncode == 0 | ||
| assert os.path.exists(output_file) | ||
| data = np.load(output_file) | ||
| print(data.files) |
| @@ -0,0 +1,69 @@ | |||
| import os | |||
| import numpy as np | |||
|
|
|||
There was a problem hiding this comment.
Remove one blank line here.
|
|
||
| str_path = str(DIR_PATH) | ||
|
|
||
|
|
There was a problem hiding this comment.
Add a test for the help, like you did for the other script.
| assert result.returncode == 0 | ||
| assert os.path.exists(output_file) | ||
| data = np.load(output_file) | ||
| print(data.files) |
| assert result.returncode == 0 | ||
| assert os.path.exists(output_file) | ||
| data = np.load(output_file) | ||
| print(data.files) |
| def test_quac_mat_adj_build(script_runner): | ||
| wm_path = str_path + "/data/simplePhantoms/fanning_2d_5bundles/wm_vf.nii.gz" | ||
| fods_path = str_path + "/data/simplePhantoms/fanning_2d_5bundles/fods.nii.gz" | ||
| output_file = "test_output_graph.npz" |
There was a problem hiding this comment.
If the output file is always the same for every test, I think you could move this line outside the functions, with DIR_PATH.
creation of basic pytest files and minor changes to utils,filter, optimization_loop