Stop using a tempfile for data reorientation

[joshuacwnewton]

Checklist

GitHub

• I've given this PR a concise, self-descriptive, and meaningful title
• I've linked relevant issues in the PR body
• I've applied the relevant labels to this PR
• I've applied a release milestone (major, minor, patch) in line with Semantic Versioning guidelines
• I've assigned a reviewer

PR contents

• I've consulted SCT's internal developer documentation to ensure my contribution is in line with any relevant design decisions
• I've added relevant tests for my contribution
• I've updated the relevant documentation for my changes, including argparse descriptions, docstrings, and ReadTheDocs tutorial pages

Description

This PR deletes an unused temporary file in order to avoid overwriting the same file repeatedly. Without doing this, on Windows machines, the following error is thrown:

FileExistsError: [WinError 183] Cannot create a file when that file already exists

I don't think the intermediate file is used for anything, so I think this is OK to do.

I wonder if using a temporary file is even necessary, though? Why can't we just reorient in-place? 🤔

Linked issues

Fixes #3714.

[codecov]

Codecov Report

Merging #3727 (6dfd3ba) into master (f26377c) will decrease coverage by 0.00%.
The diff coverage is 100.00%.

Flag	Coverage Δ
api-tests	22.29% <0.00%> (+<0.01%)	⬆️
cli-tests	58.73% <100.00%> (-0.01%)	⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files	Coverage Δ
spinalcordtoolbox/scripts/sct_analyze_texture.py	82.38% <100.00%> (-0.10%)	⬇️

[mguaypaq]

I agree with your question, "Why can't we just reorient in-place?"
Looking at Image.change_orientation(), it returns a new Image object with an empty path field (as documented):

spinalcordtoolbox/spinalcordtoolbox/image.py

Lines 1061 to 1063 in 84060e2

	if im_dst is None:
	im_dst = im_src.copy()
	im_dst._path = None

So I think you could get rid of the rename, but you do have to specify the filename when saving the re-oriented copy.

Alternatively, the docstring for Image.change_orientation explicitly suggests setting im_dst to the input image for in-place operation. From skimming the code, I don't see any obvious reason why this would fail, so that might be the best option.

(As to "why is the code written this way", this business about renaming the file dates back to the initial commit from 5 years ago, so I don't know.)

[joshuacwnewton]

Looking at Image.change_orientation(), it returns a new Image object with an empty path field (as documented):

It looks like there's two change_orientation functions: The image.py function, as well as the Image class method, which calls the aforementioned function:

spinalcordtoolbox/spinalcordtoolbox/image.py

Lines 425 to 437 in 84060e2

	def change_orientation(self, orientation, inverse=False):
	"""
	Change orientation on image (in-place).
	:param orientation: orientation string (SCT "from" convention)
	:param inverse: if you think backwards, use this to specify that you actually\
	want to transform *from* the specified orientation, not *to*\
	it.
	"""
	change_orientation(self, orientation, self, inverse=inverse)
	return self

And, it looks like the method supplies self for im_dst. So, I think 73c7880 should be sufficient?

[mguaypaq]

This looks so clean! Thanks 😄