| msg194260 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-03 13:45 |
| This python is compiled with --with-pydebug option. [sky@localhost cpython]$ cat /tmp/a.txt manly man likes cute cat. [sky@localhost cpython]$ ./python Python 3.4.0a0 (default:e408e821d6c8, Jul 27 2013, 10:49:54) [GCC 4.7.2 20121109 (Red Hat 4.7.2-8)] on linux Type "help", "copyright", "credits" or "license" for more information. >>> from formatter import test >>> test('/tmp/a.txt') manly man likes cute cat. __main__:1: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/a.txt' mode='r' encoding='UTF-8'> >>> [sky@localhost cpython]$ ./python Lib/formatter.py /tmp/a.txt manly man likes cute cat. Lib/formatter.py:445: ResourceWarning: unclosed file <_io.TextIOWrapper name='/tmp/a.txt' mode='r' encoding='UTF-8'> test() |
|
|
| msg194261 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-03 13:51 |
| Sorry, I forgot about stdin. Attached the patch to handle stdin gracefully. |
|
|
| msg194262 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-03 14:04 |
| I guess I should not close stdin just in case people are using test function in the script. Attached the patch to only close the open files not stdin. |
|
|
| msg194265 - (view) |
Author: Martijn Pieters (mjpieters) * |
Date: 2013-08-03 14:44 |
| Why is the `formatter` module still part of Python 3? This was a dependency for the `htmllib` module in Python 2 only, and that module was deprecated and removed from Python 3. |
|
|
| msg194771 - (view) |
Author: STINNER Victor (vstinner) *  |
Date: 2013-08-09 20:35 |
| "Why is the `formatter` module still part of Python 3? This was a dependency for the `htmllib` module in Python 2 only, and that module was deprecated and removed from Python 3." The formatter module was deprecated? When? |
|
|
| msg194925 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2013-08-12 04:53 |
| Please wrap the "businesss" part of the test function in a try... finally. Otherwise the patch looks good. |
|
|
| msg194929 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-12 07:18 |
| Thanks, Benjamin, for reviewing my patch. Attached the fourth patch to wrap the "business" part inside the try ... finally. |
|
|
| msg194930 - (view) |
Author: Benjamin Peterson (benjamin.peterson) * |
Date: 2013-08-12 07:32 |
| That will fail if fp is not assigned before an exception is raised. I mostly mean the part starting with the for loop. |
|
|
| msg194931 - (view) |
Author: Vajrasky Kok (vajrasky) * |
Date: 2013-08-12 08:15 |
| Ah, sorry about that. Attached the fifth patch to only wrap "for loop" section inside the try... finally. |
|
|
| msg194946 - (view) |
Author: Martijn Pieters (mjpieters) * |
Date: 2013-08-12 11:37 |
| > The formatter module was deprecated? When? It wasn't, that's the point I am raising. The `formatter` module was exclusively used by the `htmllib` module, I am surprised the `formatter` module wasn't part of that deprecation. |
|
|
| msg194947 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2013-08-12 11:42 |
| Pydoc uses DumbWriter. |
|
|
| msg215278 - (view) |
Author: A.M. Kuchling (akuchling) * |
Date: 2014-03-31 22:37 |
| Version 5 of the patch looks fine; Vajrasky, I think you can go ahead and commit it. (I didn't understand RDM's comment about pydoc using DumbWriter; in 3.4, it doesn't seem to me that pydoc does. What am I missing?) |
|
|
| msg215316 - (view) |
Author: R. David Murray (r.david.murray) * |
Date: 2014-04-01 13:37 |
| There were a bunch of changes to pydoc in 3.4, so I'm not surprised that it doesn't use DumbWriter any more (possibly as part of the formatter pending deprecation). I think I was answering why it wasn't deprecated as part of the htmllib deprecation. |
|
|
| msg233442 - (view) |
Author: Roundup Robot (python-dev)  |
Date: 2015-01-05 07:18 |
| New changeset f859a61f5853 by Berker Peksag in branch '3.4': Issue #18644: Fix a ResourceWarning in formatter.test(). https://hg.python.org/cpython/rev/f859a61f5853 New changeset f374e4e6d04b by Berker Peksag in branch 'default': Issue #18644: Fix a ResourceWarning in formatter.test(). https://hg.python.org/cpython/rev/f374e4e6d04b |
|
|
| msg233443 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2015-01-05 07:20 |
| Thanks for the patch, Vajrasky. |
|
|
| msg233446 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2015-01-05 08:29 |
| Not that I think it's worth changing for this case, but I find code like this better written as: if some_test: fl = contextlib.closing(open(sys.argv[1])) else: fl = sys.stdin with fl as fl: do_stuff(fl) This way you don't need another test, the close isn't far away from the open, and you save some lines of boilerplate. Or, if "with fl as fl" bothers you: with sys.stdout if some_test else contextlib.closing(open(sys.argv[1])) as fl: do_stuff(fl) I don't recommend that, though. In any event, thanks for the fix! |
|
|
| msg233451 - (view) |
Author: Berker Peksag (berker.peksag) * |
Date: 2015-01-05 09:53 |
| Thanks for the suggestion, Eric. contextlib.closing(open(...)) looks unnecessary to me. Here is an alternative patch. |
|
|
| msg233452 - (view) |
Author: STINNER Victor (vstinner) * |
Date: 2015-01-05 09:57 |
| > fl = contextlib.closing(open(sys.argv[1])) I prefer the current code with the explicit close() and "is not sys.stdin", it's less magic :-) |
|
|
| msg233455 - (view) |
Author: Eric V. Smith (eric.smith) * |
Date: 2015-01-05 10:13 |
| Good point on contextlib.closing not being needed. I usually use this pattern on things that aren't files! On second thought, the with statement will close sys.stdin, so this isn't a valid pattern here. Sorry for the noise. |
|
|
| msg233457 - (view) |
Author: Serhiy Storchaka (serhiy.storchaka) * |
Date: 2015-01-05 10:16 |
| I prefer the current code (i.e. formatter_fix_resource_warning_v5.patch). In more complex case ExitStack can be used, but here it looks redundant. with contextlib.ExitStack() as stack: if some_test: fl = open(sys.argv[1]) stack.enter_context(fl) else: fl = sys.stdin do_stuff(fl) or if some_test: cm = fl = open(sys.argv[1]) else: fl = sys.stdin cm = contextlib.ExitStack() with cm: do_stuff(fl) |
|
|