Deep-MI / BrainPrint

A python-only derivative of the original BrainPrint scripts
MIT License
11 stars 9 forks source link

Brainprint PyTests #19

Closed engrosamaali91 closed 7 months ago

engrosamaali91 commented 1 year ago

Method to run brainprint test cases using pytest:

codecov[bot] commented 1 year ago

Codecov Report

Merging #19 (cd606cd) into main (1b89f5a) will not change coverage. Report is 9 commits behind head on main. The diff coverage is n/a.

:exclamation: Current head cd606cd differs from pull request most recent head f8a5e9d. Consider uploading reports for the commit f8a5e9d to get more accurate results

@@           Coverage Diff           @@
##             main      #19   +/-   ##
=======================================
  Coverage   26.62%   26.62%           
=======================================
  Files           8        8           
  Lines         353      353           
  Branches       43       43           
=======================================
  Hits           94       94           
  Misses        256      256           
  Partials        3        3           
Flag Coverage Δ
unittests 26.62% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Coverage Δ
brainprint/brainprint.py 20.58% <ø> (ø)
brainprint/surfaces.py 20.37% <ø> (ø)

:mega: We’re building smart automated test selection to slash your CI/CD build times. Learn more

m-reuter commented 1 year ago

Thanks, can you:

engrosamaali91 commented 1 year ago

In this commit cd606cd only errors were resolved. The tests have not been moved to brainprint/utils/test yet.

engrosamaali91 commented 1 year ago

Apparently f8a5e9d automated tests are not possible on github as it requires FREESURFER export and subject directory as well. However the pytests for brainprint can be done locally.

m-reuter commented 1 year ago

Yes, but are there any tests that can be done without FreeSurfer to be installed? We would not want to install all of FreeSurfer as that is too large and takes too long. But we can replace several of the freesurfer binaries with our own python scripts, for example mri_binarize just combines labels, and there is also code for marching-cube (to extract surfaces), that could increase the number of tests that do not need FreeSurfer.

kdiers commented 1 year ago

I think @m-reuter's comment touches the more general question how independent of FreeSurfer this software can be already. To my understanding, the only FreeSurfer binaries that we use are mri_binarize, mri_pretess, mri_mc, and mris_convert (all of them called in surfaces.py). Not exactly sure about mri_pretess (and if it's strictly needed), but I think all others could be replaced by python code. We might not be able to exactly reproduce FreeSurfer-based results, but I guess results obtained without FreeSurfer-based code would be valid as well. In that sense, it might be rather an issue of compatibility between versions of this software. Moving away from FreeSurfer would require some development and validation, but if we want to do this, then it would also allow to run all tests remotely - not just some.

kdiers commented 7 months ago

Redirecting the pull request from the previous target branch (main) to a new branch (add-tests), because the pull request still needs to be modified, but pushing to the external, forked repository is no longer desired. Will eventually merge the pull request into the new target branch, do necessary modifications, and after that create another PR back into the main branch.