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

[ENH] Coerce images to 32-bit #759

Merged
merged 1 commit into from
Jul 19, 2021
Merged

[ENH] Coerce images to 32-bit #759

merged 1 commit into from
Jul 19, 2021

Conversation

jbteves
Copy link
Collaborator

@jbteves jbteves commented Jul 19, 2021

Closes #757 .

Changes proposed in this pull request:

  • Downscales 64-bit float, int arrays to 32-bit arrays just before image save
  • Adds some notes to the io.save_img docstring

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The changes you've made look good to me. I have one question/recommendation, but I was also thinking maybe we should have something in the outputs page about the change in datatypes?

tedana/io.py Show resolved Hide resolved
@codecov
Copy link

codecov bot commented Jul 19, 2021

Codecov Report

Merging #759 (a52af7b) into main (12f16eb) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #759      +/-   ##
==========================================
+ Coverage   92.83%   92.85%   +0.01%     
==========================================
  Files          27       27              
  Lines        2192     2197       +5     
==========================================
+ Hits         2035     2040       +5     
  Misses        157      157              
Impacted Files Coverage Δ
tedana/io.py 93.36% <100.00%> (+0.17%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 12f16eb...a52af7b. Read the comment docs.

@jbteves
Copy link
Collaborator Author

jbteves commented Jul 19, 2021

Hm, I'm not sure what we'd put in the outputs page. This change should be relatively invisible aside from freer disks 🙂. Programs should read everything correctly, and float32 is the most widely supported data type of them all. I think only developers need to be aware, but maybe there's something I'm not thinking of.

Copy link
Member

@tsalo tsalo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's fair. I guess not many people will care. Approving!

@handwerkerd handwerkerd added the bug issues describing a bug or error found in the project label Jul 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug issues describing a bug or error found in the project
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Integer-based output images are unreadable with AFNI
3 participants