Skip to content

Conversation

@Lestropie
Copy link
Member

#2086 has become bloated showing commits that are unrelated, probably because it was initially configured as a merge to dev. But I do think I'd consider this a bug, so I've done a clean cherry-pick and made a new PR.

@Lestropie Lestropie added the bug label Jun 24, 2020
@Lestropie Lestropie added this to the 3.0.1 milestone Jun 24, 2020
@Lestropie Lestropie requested a review from a team June 24, 2020 03:15
@Lestropie Lestropie self-assigned this Jun 24, 2020
If no output image path is specified to mrthreshold, and therefore it will be writing the determined threshold values to the standard output stream, then progress bars should not be displayed on the terminal, as these can overwrite the values written to standard output.
@Lestropie
Copy link
Member Author

# MASTER; WITH OUTPUT IMAGE
[rob@rsmithmetabox diffusion]$ ~/src/master/bin/mrthreshold ev.mif master.mif 
mrthreshold: [100%] Determining and applying per-volume thresholds

# PR CODE; WITH OUTPUT IMAGE
[rob@rsmithmetabox diffusion]$ mrthreshold ev.mif pr.mif 
mrthreshold: [100%] Determining and applying per-volume thresholds

# MASTER; NO OUTPUT IMAGE
[rob@rsmithmetabox diffusion]$ ~/src/master/bin/mrthreshold ev.mif 
mrthreshold: [ 33%] Determining and applying per-volume thresholds...
mrthreshold: [ 67%] Determining and applying per-volume thresholds...
mrthreshold: [100%] Determining and applying per-volume thresholds

# PR CODE; NO OUTPUT IMAGE
[rob@rsmithmetabox diffusion]$ mrthreshold ev.mif 
0.142654
-0.134121
-0.132325

Copy link
Member

@jdtournier jdtournier left a comment

Choose a reason for hiding this comment

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

Looks good to me...

@jdtournier jdtournier merged commit 65a320a into master Jun 24, 2020
@jdtournier jdtournier deleted the mrthreshold_cout_fix_master branch June 24, 2020 10:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants