[Python-ideas] chdir context manager (original) (raw)

Terry Reedy tjreedy at udel.edu
Sat Jan 19 14:37:17 CET 2013


On 1/19/2013 5:10 AM, Daniel Shahaf wrote:

The following is a common pattern (used by, for example, shutil.makearchive):

savecwd = os.getcwd() try: foo() finally: os.chdir(savecwd) I suggest this deserves a context manager: with savedcwd(): foo()

This strikes me as not a proper context manager. A context manager should create a temporary, altered context. One way is to add something that is deleted on exit. File as context managers are the typical example. Another way is to alter something after saving the restore info, and restoring on exit. An example would be a context manager to temporarily change stdout. (Do we have one? If not, it would be at least as generally useful as this proposal.)

So to me, your proposal is only 1/2 or 2/3 of a context manager. (And 'returns an open file descriptor for the saved directory' seems backward or wrong for a context manager.) It does not actually make a new context. A proper temp_cwd context manager should have one parameter, the new working directory, with chdir(new_cwd) in the enter method. To allow for conditional switching, the two chdir system calls could be conditional on new_cwd (either None or '' would mean no chdir calls).

Looking at your pattern, if foo() does not change cwd, the save and restore is pointless, even if harmless. If foo does change cwd, it should also restore it, whether explicitly or with a context manager temp_cwd.

Looking at your actual example, shutil.make_archive, the change and restore are conditional and asymmetrical.

save_cwd = os.getcwd() if root_dir is not None: ... if not dry_run: os.chdir(root_dir) ... finally: if root_dir is not None: ... os.chdir(save_cwd)

The initial chdir is conditional on dry_run (undocumented, but passed on to the archive function), the restore is not. Since I believe not switching on dry_runs is just a minor optimization, I believe that that condition could be dropped and the code re-written as

with new_cwd(root_dir): ...

I am aware that this would require a change in the finally logging, but that would be true of the original proposal also.

-- Terry Jan Reedy



More information about the Python-ideas mailing list