Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New scripts for conversion, comparison and testing #22

Merged
merged 15 commits into from
May 3, 2022
Merged

Conversation

frheault
Copy link
Collaborator

@frheault frheault commented Apr 5, 2022

4 new scripts (python) that will help to go back and forth between formats.
tff_verify_header_compatibility.py For NIFTI, TRK and TRX only, compares header informations (affine, dimensions, voxel sizes and voxel order) and inform the user about their compatibility
tff_simple_compare.py Subtract two tractography files in RASMM, VOXMM and VOX and inform the user about their similarity (point to point distance). Does not support tractograms with different streamline count or streamline shuffling (very simple comparison)
tff_visualize_overlap.py Load a tractogram, compute density maps (using 2 approach in Dipy) and render the density map and its bounding box (see below). 3 Rendering will be displayed (RASMM, VOXMM, VOX). If there is an incompatibility between the header and the coordinates the density map computation will crash OR the tractogram won't appear in the right position.
tff_convert_tractogram.py Script to convert from one to another file format (.trk, .tck, .vtk, .fib, .dpy, .trx)

  • It is expected that a trx/trk should have a matching header with a NIFTI file from which they were generated. tff_verify_header_compatibility.py should help to verify this.
  • Conversion from any file format to another should not make tff_simple_compare.py fail
  • mrview or MI-Brain should be used to double-check if tck/trk align with their respective FA (both only work in RASMM)

image

Files and commands to test this PR:
https://drive.google.com/file/d/1vt9GWy--gRU5sYfgfe-qgFQyN2DHYyQQ/view?usp=sharing

@frheault frheault mentioned this pull request Apr 5, 2022
@arokem
Copy link
Collaborator

arokem commented Apr 6, 2022

Overall, looks great! One general comment/question: would it be possible to implement these functionalities as library code and then make the scripts import and execute this library code? I think it will make it useful in other contexts, easier to test, and also might help reduce some boiler-plate. For example, it would be good to have a supported_formats variable implemented as a module-global variable and test inputs against this variable, so changes only have to be introduced one place, and so on.

@frheault
Copy link
Collaborator Author

frheault commented Apr 7, 2022

@arokem So just to be sure, you would put 100% of the main inside of a function and make that function receive only the string (filenames) as parameters? And the behavior would stay the same?

@arokem
Copy link
Collaborator

arokem commented Apr 7, 2022

you would put 100% of the main inside of a function and make that function receive only the string (filenames) as parameters? And the behavior would stay the same?

Yeah - that would be good. If you put them all in the same module, you can start consolidating some of the boiler-plate, e.g., defining supported file-types and checking inputs and so on, and use shared test harnesses (eventually)

@frheault
Copy link
Collaborator Author

frheault commented May 3, 2022

@arokem Is this ok for now? The visualization scripts are not tested because they are just rendering.
But the conversion is tested and datatype too. That's the most important part first.

@arokem
Copy link
Collaborator

arokem commented May 3, 2022

Yeah - I think that we can go ahead and merge this.

I have some ideas for subsequent cleanup and tweaks that we can do -- I'll post some issues.

But for now, let's merge this and move ahead with this.

@arokem arokem merged commit f3ebd3e into master May 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants