-
Notifications
You must be signed in to change notification settings - Fork 27
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
A python script to wrap image read and write for Nifti Images. #60
Conversation
Does it make more sense with my comments? |
@IvanARashid This could use your multi-voxel fitting. Also, if you have ideas about how to best merge with the wrapper and handle inputs/outputs of the wrapper, that would be helpful. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can test it with some nifti files. There are nifti files in our zenodo data repository you can download. You might need to merge them to create a 4D to fully test though.
Regarding the fitting of multiple voxels. Yes, I was working on a solution for this in the wrapper, although that work was stranded due to the previous git issues. I won't be able to revisit this for a few weeks, but the solution that @etpeterson suggested is a fine placeholder for now. I.e. loop over all the voxels and fit them individually. Once I've implemented this functionality into the wrapper, we can get rid of it in this function. When this functionality is in the wrapper, the returned result will be parametric map volumes, i.e 2D or 3D images of parameter values. So you could try to implement something similar now. Or we could hold this PR until multi voxel fitting is in the wrapper. |
I would definitely recommend testing it like this, I think it would make things much more clear. It looks like the abdomen or the brain nifti files ( |
Thank for the review, the signal variable is still missing ? |
In the code I sent the |
I now, understand, I will also test the script now also, I also got to know that the algorithm are located at |
Just checking on this. Is it waiting on me? |
No @etpeterson, I wanted to test it out and see it workings. I will do that. Recently had lot of different school activites. |
Ok, no problem. Just wanted to make sure. |
@etpeterson I downloaded the Data from the link, I was trying to test it out. Checking the content of brain.bval and brain.bvec, I added the two function read_bval_file and read_bval_file as they were not json.
I am current using the brain.nii.gz, but I Keep getting this error below. I am using the algorithms in the src/standardized. I also changed the algorithm couple of time also, but it keep giving almost similar error. |
@etpeterson bringing your attention to this. |
I would guess that the bvals and bvecs aren't numpy arrays or are the wrong size. Maybe try transposing them? It could also be the input data are not the right size. The data length should match the bvalue length. I'm just returning from a break so I could try to debug it later this week if you're still having trouble. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it's close
@etpeterson thanks, I will look into this. |
@etpeterson, kindly check this out. Thanks. And also welcome back from the break. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. Just 2 small comments. Nice work.
@etpeterson I also do not know why some of the test is failing, I checked it. But, it does not seem related with my changes. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just the one comment.
Not your fault, it looks like github changed something that broke older python versions. I'm not sure if it's going to be fixed or not, but let's leave it for a day or two to see if it comes back. |
Ok, this was fixed. Just the one change that I see remaining. |
@etpeterson, the tqdm works by having the information about the number of iteration beforehand. But, in our code. We are using yield function which would not give the information about the number of iteration beforehand. The progress bar will not be displayed if the number of iteration is not known. The solution is to provide a number(if known). Also, if we can not get the number of iteration. We can basically print when it is starting and when it is ending. |
We do know the number, it's almost the same code as the generator. Also, it looks like if you wrapped it up into a class it could query that implicitly. https://stackoverflow.com/questions/41985993/tqdm-show-progress-for-a-generator-i-know-the-length-of |
…ch is gotten from the image itself
@etpeterson, I already make the value passed to the total argument dynamic. |
Looks good to me, thanks! |
Describe the changes you have made in this PR
I am using this script as the placeholder for the final script, we can continue the discussion from here.
@etpeterson
I have quite not understand the available algorithm and where they are located. Kindly give more information. Thank you.
Fixes #58
Checklist