ME-ICA / aroma

ICA-AROMA, as a Python package. A work in progress.
Apache License 2.0
7 stars 11 forks source link

[TST] Redo unit tests for feature calculations and add data to compare with #34

Closed eurunuela closed 3 years ago

eurunuela commented 4 years ago

Closes #32 .

Changes proposed in this pull request:

vinferrer commented 3 years ago

Just a couple of minor things. Also, can you add the script used to generate the test files?

The test files where generated by applying the pipeline with the fixed seed. One thing we could try is compare the result of the function with the FSL-AROMA equivalent, but that would be an integration test rather than unittest

eurunuela commented 3 years ago

The test files where generated by applying the pipeline with the fixed seed. One thing we could try is compare the result of the function with the FSL-AROMA equivalent, but that would be an integration test rather than unittest

This is something @tsalo and I talk about yesterday. He wants (and I agree) to have the exact command used to generate the datasets. We could add it as a comment in the docstring for instance.

eurunuela commented 3 years ago

Let me know what you think @tsalo @vinferrer

notZaki commented 3 years ago

Looks fine to me.

Only potential change I can think of is deciding if binary data files should be included in the repository or downloaded during tests. Right now the repository is 0.2 MiB of code, and 23 MiB of test data.

eurunuela commented 3 years ago

Only potential change I can think of is deciding if binary data files should be included in the repository or downloaded during tests. Right now the repository is 0.2 MiB of code, and 23 MiB of test data.

Good point @notZaki . I've opened #44 to do that. Thanks!