-
-
Notifications
You must be signed in to change notification settings - Fork 31k
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
Allow delete=False
for tempfile.TemporaryDirectory()
#100131
Comments
Just use The purpose of the context manager is automatic deletion of the directory when you leave the block. If you do not need this, you do not need a contect manager. |
there's one additional way to archive this: with (
contextlib.nullcontext(tempfile.mkdtemp())
if args.keep_temp_files
else tempfile.TemporaryDirectory()
) as dir:
do_something(dir) |
I know that. The
This is not suitable, if you are not a Python expert |
I'm going to close this based on Serhiy's practical |
The point of |
FWIW, given the basic stated motivation for this feature, both on this issue and the previous one, is cases in which the cleanup may or may not be required, the decent number of users who presented similar use case, it's not clear to me how "just use Sure, there are workarounds, but the OP illustrates the issues with each, and in summary the obvious ones are kludgy and the non-kludgy ones are not obvious. It also seems like it could increase the chance users reach for the more cryptic lower-level interfaces for the other cases where the higher-level one would be safer, simpler and more ergonomic. In light of the fact that the code changes to the stdlib are relatively minimal, and there's an up to date PR with tests and docs provided, I'm inclined to agree with @zware that this change shouldn't be rejected on that basis. |
fair enough. I really don't think this feature will see much use, but I've used the PR to improve the docstring anyways. |
It still feels quite odd to me to say "We have a There's been a number of different ways of doing this pointed out in the above thread, but FWIW I'd probably use a with ExitStack() as stack:
if args.keep_temp_files:
dir = stack.enter_context(TemporaryDirectory())
else:
dir = mkdtemp()
do_something(dir) |
I don't think arguing on the basis of the meaning of "temporary" holds much water here. The ExitStack solution and the nullcontext solution are both just fine, but they are quite a bit more verbose. The use-case that I find most convincing is the "temporary (ahem) debugging" use case, where you have a TemporaryDirectory that you generally do want cleaned up, but in the course of some debugging you need to leave it around to look at. I've definitely had this situation, and reindenting a bunch of code or adding a new import for |
It may seem odd at first glance, but there are even non-debugging uses - you may have code that needs cleanup to be conditional based on user input. This, perhaps unusual, feature provides an easy way to do that without adopting unusual code structures. I added the missing documentation to the PR and described this in there as I expect others to look at the feature with the same surprise I initially had as well. |
…ry() (#100132) Add optional delete parameter to tempfile.TemporaryDirectory(). Co-authored-by: Gregory P. Smith <greg@krypto.org>
Thanks all, including the contributed PR and the discussion. Merged. You changed my mind. :) |
…irectory() (python#100132) Add optional delete parameter to tempfile.TemporaryDirectory(). Co-authored-by: Gregory P. Smith <greg@krypto.org>
…irectory() (python#100132) Add optional delete parameter to tempfile.TemporaryDirectory(). Co-authored-by: Gregory P. Smith <greg@krypto.org>
Feature or enhancement
If you use
tempfile.TemporaryDirectory()
, the directory is automagically deleted, what is the whole purpose of this class. But sometimes you may want to take a look at it for debugging reasons, so it would be nice to have adelete=False
to prevent deleting.Pitch
Code using this could look like this:
I personal use
tempfile.TemporaryDirectory()
a lot, especial in build scripts. Sometimes a need to take a look at the content of these. so having this Option would be great for that.Other ways to achieve this
Of course, there would be other ways to achieve this.
1. Use a extra function that takes the directory as argument:
This would need some code rewrite.
2. Using
input()
This could cause problems in situations, where you can't easily give some Keyboard input to TTY.
3. Using a custom function
This will blow up your code when you just write a simple script. Python also often used by beginners, which may don't know, they can do this and start refactoring their code.
Why you should implement this
The argument can be easy added to exiting scripts if needed, without any additional work. A scripting language like Python should allow this to make debug more easy and more beginner friendly.
It is absolutely no work to implement this. If this would be much work to implement and maintain this, I would not suggested it. But in this case, it's just 3 steeps:
__init__()
:__exit__()
ask, if it should delete or notPrevious discussion
None
Linked PRs
The text was updated successfully, but these errors were encountered: