-
-
Notifications
You must be signed in to change notification settings - Fork 20
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
update python to go along with matlab changes in 9beee53 #278
update python to go along with matlab changes in 9beee53 #278
Conversation
Looks good to me. Thanks for investigating this! |
@noahg-neuroimage are you sure it looks good? The outputs are different now between Matlab and Python when running |
Oh, it seems I misinterpreted the information shown above, haha. |
got to do with fread not the code does do fread(data,int16) but fread(specificdataframes,int16>=int16) which if I understand forces the read data into 16 bit range as oppose to read a 16 bit data frame but values can be out of range -- in python, i think is something like Convert.ToInt16() vs (Int16) |
…-bug-fbp-scaling-negative-values-error
It seems like as I more closely approach the matlab code in read_ecat and ecat2nii the image quality of the python images continues to decrease.
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 always a good idea to set endianity
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 always a good idea to set endianity
The scaling is needed because Often data range is 12 bits and not 16 (do not ask why)
I don't disagree, but I'm not happy with the current results as:
There's something I'm failing to account for in this PR. |
…xt for arrays since they're different than cell arrays.
* Use more idiomatic way of constructing an affine * Remove unnecessary header zeroing, use helper methods where appropriate
Seems like we were most of the way there, but added some small fixes that @effigies pointed out, merged his PR, and compared the differences between the output nifitis for the FBP images that caused all this ruckus in the first place. Subtracting the matlab and python niftis led to a negligible mean difference:
And nib-diff confirms:
|
…ub.com:openneuropet/PET2BIDS into 272-ecat-bug-fbp-scaling-negative-values-error
# Conflicts: # .gitignore # pypet2bids/pypet2bids/ecat2nii.py # pypet2bids/pypet2bids/read_ecat.py
fixed % and added rms Co-authored-by: Chris Markiewicz <[email protected]>
Co-authored-by: Chris Markiewicz <[email protected]>
Something broke the matlab CI, hoping it's just a file getting unzipped and not being able to be found.... |
Merging this as the matlab unit tests are working fine when run locally, going to create a new issue to fix that fun bug. @noahg-neuroimage I believe this has been fixed now |
thx for the help @effigies ! this was a bit of a struggle to figure out what was going on |
Having flashbacks with this fix for issue #272
📚 Documentation preview 📚: https://pet2bids--278.org.readthedocs.build/en/278/