Created on 2012-01-19 12:25 by David.Layton, last changed 2012-07-21 21:01 by bethard.
|msg151615 - (view)||Author: David Layton (David.Layton)||Date: 2012-01-19 12:25|
argparse.FileType.__call__ opens the specified file and returns it. This is well documented as an anit-idiom in http://docs.python.org/howto/doanddont.html#exceptions. "...a serious problem — due to implementation details in CPython, the file would not be closed when an exception is raised until the exception handler finishes; and, worse, in other implementations (e.g., Jython) it might not be closed at all regardless of whether or not an exception is raised." Disregarding the above, handling a file which may or may not have been opened depending the users input requires a bit of boilerplate code compared to the usual with-open idiom. Additionally, there is no way to prevent FileType from clobbering an existing file when used with write mode. Given these issues and others, it seems to me that the usefulness of FileType is outweighed by propensity to encourage bad coding. Perhaps, it would be best if FileType (or some replacement) simply checked that the file exists (when such a check is appropriate), it can be opened in the specified mode, and, curry the call to open (i.e. return lambda: open(string, self._mode, self._bufsize))
|msg152518 - (view)||Author: Éric Araujo (eric.araujo) *||Date: 2012-02-03 14:32|
> Additionally, there is no way to prevent FileType from clobbering an existing file > when used with write mode. I think this deserves its own feature request: now that Python 3.3 has an “exclusive create” mode, argparse.FileType could gain support for it. Would you open that request? > Given these issues and others, We have one issue and one missing feature; what are the other issues? > it seems to me that the usefulness of FileType is outweighed by propensity to encourage bad > coding. I think the problem is not as bad as you paint it. A great number of unclosed file handles may cause problem to some OSes, but that’s it. On the plus side, the fact that argparse accepts for its type argument any callable that can check and convert a string input is simple, clean and works. FileType is needed. In packaging/distutils2 for example we have similar functions that return an open file object and never close it: the responsibility is at a higher level. Other packaging code calling these functions does so in a with statement. It is not evil by nature. The problem here is that FileType may return stdin or stdout, so we can’t just always close the file object (or maybe we can, say using an atexit handler?). > Perhaps, it would be best if FileType (or some replacement) simply checked that the file exists But then you’d run into race conditions. The only sure was to say if a file can be opened is to open it.
|msg152519 - (view)||Author: Éric Araujo (eric.araujo) *||Date: 2012-02-03 14:37|
s/sure was/sure way/
|msg152530 - (view)||Author: David Layton (David.Layton)||Date: 2012-02-03 15:59|
Eric, checked that the file exists But then you’d run into race conditions. The only sure was to say if a file can be opened is to open it. I think you misunderstand me. I am NOT suggesting that you open and close the file. I am saying that you should not open it in the first place. If I cannot open the file at the point in my program where I actually want to open it, then fine, I can decide, in my own code, what to do. propensity to encourage bad > coding. I think the problem is not as bad as you paint it. A great number of unclosed file handles may cause problem to some OSes, but that’s it. On the plus side, the fact that argparse accepts for its type argument any callable that can check and convert a string input is simple, clean and works. FileType is needed. Causing a problem on some OSes and not others is worse than causing a problem on all OSes as it increases the likelihood of buggy code passing tests and moving to production. I think argparse is wonderful; I just think that by having FileType not open the file, the number of it's use cases is increased. As it stands now, I would prefer the just pass the argument as a string argument and handling the opening myself unless: 1. I wanted my program to open the file at the very beginning of the program (where one traditionally handles arg parsing) 2. I wanted to exit on the first, and only, attempt to open the file 3. I really did not care if the file closed properly--which, granted, is often the case with tiny scripts The moment any of these is not true due to a change in requirements, I will have to refactor my code to use a filename arg. Where as if I start out with a bog-standard filename and open it myself, I can easily add the behaviour I want. I just don't see FileType as a big convenience. However, I do see that changing this would break backwards compatibility and would not want to see that happen. Perhaps a new FileNameType that does some basic, perhaps, optional checks would have wider use-cases. I hope this helps. David Layton On Fri, Feb 3, 2012 at 2:37 PM, Éric Araujo <firstname.lastname@example.org> wrote: > > Éric Araujo <email@example.com> added the comment: > > s/sure was/sure way/ > > ---------- > > _______________________________________ > Python tracker <firstname.lastname@example.org> > <http://bugs.python.org/issue13824> > _______________________________________ >
|msg166068 - (view)||Author: Steven Bethard (bethard) *||Date: 2012-07-21 21:01|
So I generally agree that FileType is not what you want for anything but quick scripts that can afford to either leave a file open or to close stdin and/or stdout. However, quick scripts are an important use case for argparse, so I don't think we should get rid of FileType. What should definitely happen: * Someone should add some documentation explaining the risks of using FileType (e.g. forgetting to close the file, or closing stdin/stdout when you didn't mean to). What could potentially happen if someone really wanted it: * Someone could create a "safe" replacement, e.g. a FileOpenerType that returns an object with open() and close() methods that do the right things (or whatever API makes sense).
|2012-07-21 21:01:09||bethard||set||messages: + msg166068|
|2012-02-03 15:59:12||David.Layton||set||messages: + msg152530|
|2012-02-03 14:37:42||eric.araujo||set||messages: + msg152519|
messages: + msg152518
title: argparse.FileType opens a file without excepting resposibility for closing it -> argparse.FileType opens a file and never closes it