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

dwifslpreproc: Handle non-ascii data in eddy error outputs #2313

Merged
merged 2 commits into from May 6, 2021

Conversation

Lestropie
Copy link
Member

The FSL eddy commands sometimes contain a small block of binary data at the end of their outputs. If, when attempting to write those data to a text file in the scratch directory, an encoding operation occurs, such data may yield a UnicodeEncodeError. This commit changes the output text files to be opened in binary mode, so such an encoding should no longer be applied.

(Note that __str__() is defined for MRtrixCmdError as a straight concatenation of stdout and stderr contents, so there shouldn't be any ascii conversion happening there)

The FSL eddy commands sometimes contain a small block of binary data at the end of their outputs. If, when attempting to write those data to a text file in the scratchh directory, an encoding operation occurs, such data may yield a UnicodeEncodeError. This commit changes the output text files to be opened in binary mode, so such an encoding should no longer be applied.
@Lestropie Lestropie added this to the 3.0.4 hotfix milestone May 6, 2021
@Lestropie Lestropie requested a review from a team May 6, 2021 05:52
@Lestropie Lestropie self-assigned this May 6, 2021
jdtournier
jdtournier previously approved these changes May 6, 2021
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.

Seems fairly innocuous to me. My experience is that using non-binary mode on text files eventually creates problems anyway, so quite happy with the suggested change.

@jdtournier jdtournier merged commit 62468e7 into master May 6, 2021
@jdtournier jdtournier deleted the dwifslpreproc_eddy_error_fix branch May 6, 2021 17:39
Lestropie added a commit that referenced this pull request May 7, 2021
- On EddyQC failure, write text file containing stdout / stderr contents in binary mode, just as is done for eddy (#2313).
- For EddyQC exports, deal with prospect of whitespace in output directory path.
Lestropie added a commit that referenced this pull request May 12, 2021
Follow-up to #2313, which has been observed to cause issues.
Relates to discussion in #2191.
@Lestropie Lestropie modified the milestones: 3.0.4 hotfix, 3.0.3 hotfix Jun 25, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants