Repository navigation
Commit b438c45
committed
Refactor TemporaryFileSwap.__init__ for clarity
This changes the mkstemp call TemporaryFileSwap uses to atomically
create (and thus reserve) the temporary file, so that it passes
only a filename (i.e., basename) prefix as `prefix`, and passes all
other path components (i.e., the directory) as `dir`. (If the
original path has no directory, then `dir` is "" as before.)
This makes the mkstemp call *slightly* more idiomatic. This also
makes it clearer, as it is no longer using `prefix` for something
that feels like it should be possible to pass as a Path object.
Although mkstemp does not accept a Path as `prefix`, it does (as
expected) accept one as `dir`. However, to keep the code simple,
I'm passing str for both. The os.path.split function accepts both
str and Path (since Python 3.6), and returns str objects, which are
now used for the `dir` and `prefix` arguments to mkstemp.
For unusual cases, this may technically not be a refactoring. For
example, a file_path of "a/b//c" will be split into "a/b" and "c".
If the automatically generated temporary file suffix is "xyz", then
that results in a tmp_file_path of "a/b/cxyz" where "a/b//cxyz"
would have been used before. The tmp_file_path attribute of a
TemporaryFileSwap object is public (and used in filesystem calls).
However, no guarantee has ever been given that the temporary file
path have the original path as an exact string prefix. I believe
the slightly weaker relationship I expressed in the recently
introduced test_temporary_file_swap -- another file in the same
directory, named with the original filename with more characters,
consisting of equivalent path components in the same order -- has
always been the intended one.
Note that this slight possible variation does not apply to the
file_path attribute. That attribute is always kept exactly as it
was, both in its type and its value, and it always used unmodified
in calls that access the filesystem.1 parent 1ddf953 commit b438c45
1 file changed
Lines changed: 2 additions & 1 deletion
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
@@ -41,7 +41,8 @@ class TemporaryFileSwap: | |||
| 41 | 41 | ||
| 42 | 42 | def __init__(self, file_path: PathLike) -> None: | |
| 43 | 43 | self.file_path = file_path | |
| 44 | - fd, self.tmp_file_path = tempfile.mkstemp(prefix=str(self.file_path), dir="") | ||
| 44 | + dirname, basename = osp.split(file_path) | ||
| 45 | + fd, self.tmp_file_path = tempfile.mkstemp(prefix=basename, dir=dirname) | ||
| 45 | 46 | os.close(fd) | |
| 46 | 47 | with contextlib.suppress(OSError): # It may be that the source does not exist. | |
| 47 | 48 | os.replace(self.file_path, self.tmp_file_path) | |
0 commit comments