msg121545 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-19 16:04 |
The docs contain numerous examples that would trigger resource warnings under 3.2 (for example “open(...).read()”). They should be changed to use (and thus promote) the with statement.
Not adding the “easy” keyword, since grepping for those things is not easy.
Not sure we’ll backport that to 3.1 and 2.7.
|
msg121559 - (view) |
Author: Alexander Belopolsky (belopolsky) * |
Date: 2010-11-19 19:10 |
+1
BTW, I've updated examples in Unicode HOWTO to use with.
|
msg121565 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2010-11-19 20:12 |
+1
I have not yet had occasion to use 'with' yet, but in reading the Unicode HOWTO diff, I noticed that I liked replacing 'open,read,close' with 'with open, read' just for reading purposes since it turns 3 steps into 1 compound transaction.
Perhaps something should also be added to the doc style guide (along with using 'attributes' instead of 'members').
|
msg121622 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-20 12:36 |
with also replaces open-try-do stuff-finally-close, the correct idiom for ensuring file handles are always closes in all VMs.
I don’t think the doc style guide is the right place, since this is a code issue. with is advertised in http://docs.python.org/dev/tutorial/inputoutput#methods-of-file-objects and http://docs.python.org/dev/howto/doanddont#exceptions ; http://docs.python.org/dev/library/functions#open says nothing about closing, and http://docs.python.org/dev/library/io tells about the with statement without recommending it strongly.
|
msg121625 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 12:38 |
Éric, although grepping for all such references may be tricky, could you specify the places where you did see them? I guess a few fixed places is better than none at all.
|
msg121627 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-20 12:56 |
Certainly. Here is my secret grep:
$ cd py3k/Doc
$ grep -n --color=auto -d skip -I --exclude-dir .svn --exclude-dir .hg -R open\(.*\).read *.rst c-api distutils documenting extending faq howto library reference tutorial using
List without false positives:
faq/library.rst
library/pkgutil.rst
library/atexit.rst
library/pipes.rst
library/cmd.rst
library/logging.rst
library/difflib.rst
library/collections.rst
tutorial/interpreter.rst
tutorial/stdlib2.rst
List for open(.*).write:
faq/library.rst
library/ftplib.rst
library/atexit.rst
There is probably a better regex that would catch “open(.*).[valid method name]”, but I hate regexes.
|
msg121630 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 13:10 |
Eric, I'm attaching a provisional fix for library/atexit.rst just to be sure this is what you mean.
|
msg121634 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-20 13:19 |
Yes, that’s it. Please don’t change whitespace in your patches.
|
msg121635 - (view) |
Author: Georg Brandl (georg.brandl) * |
Date: 2010-11-20 13:21 |
FWIW, this doesn't belong in the style guide; it is obvious that the docs should exhibit "best practice" Python code, and using the with statement when opening resources is certainly such a best practice now.
|
msg121637 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 13:25 |
Eric, which whitespace change do you refer to. I changed to 4-spaces indentation in the code sample to conform to PEP-8. Shouldn't I have?
|
msg121638 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 13:27 |
Here is the patch for Docs/library/cmd.rst
|
msg121639 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 13:42 |
Here is the patch for Doc/library/difflib.rst
|
msg121643 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 13:54 |
I'm trying to port the example in tutorial/stdlib2.rst, but the sample in "working with binary data record layouts" fails (before my porting to 'with'...)
struct.error: unpack requires a bytes argument of length 16
|
msg121644 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 13:59 |
Patch for Doc/library/collections.rst
|
msg121647 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 14:05 |
Attaching patches for library/atexit.rst and for tutorial/stdlib2.rst
|
msg121648 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 14:08 |
SilentGhost,
Your patches look fine. I have a doubt re collections.rst, however - about the Python prompt. The same issue is in faq/library.rst and I didn't want to touch it because I thought that on the prompt personally I probably wouldn't use 'with' - why write 2 lines when you can write 1? After all, it's just playing on the prompt - I don't care about safe closing of the file, etc.
|
msg121653 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 14:31 |
patch for Doc/library/logging.rst
Also, note the the change of the mode from `'r'` to `'rb'`. `data_to_send` is further send through socket and therefore requires to be bytes.
I expressed my opinion in irc, but I can repeat here that I think only the most trivial code such as in Doc/library/pipes.rst isn't worth converting.
|
msg121654 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 14:32 |
patch for Doc/library/logging.rst
Also, note the the change of the mode from `'r'` to `'rb'`. `data_to_send` is further send through socket and therefore requires to be bytes.
I expressed my opinion in irc, but I can repeat here that I think only the most trivial code such as in Doc/library/pipes.rst isn't worth converting.
|
msg121703 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-11-20 18:21 |
Do not change the examples in collections.rst.
That would interfere with the clarify of what
is being demonstrated. For example, the hamlet.txt
example is not about file reading, it about
counting words.
|
msg121706 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-11-20 18:24 |
None of the changes are about file reading, they only about using with statement throughout. May be nofix then?
|
msg121708 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-11-20 18:28 |
I think the other (non collections patches) are fine. The change doesn't break up the flow of the text.
|
msg121710 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-11-20 18:31 |
Separate note for Éric: the try/finally examples do not need to change. Those are valid python. Users need to learn both try/finally and the with-statement.
|
msg121713 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-20 18:44 |
Eli, SilentGhost: Please open other bug reports for doc errors like the struct one in stdlib2 or the r/rb one.
SilentGhost: with statement used with open *is* about file reading :)
Raymond: I agree about try/finally. I agree with Alexander about collections.rst.
|
msg121730 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-20 19:20 |
On Sat, Nov 20, 2010 at 20:44, Éric Araujo <report@bugs.python.org> wrote:
>
> Éric Araujo <merwok@netwok.org> added the comment:
>
> Eli, SilentGhost: Please open other bug reports for doc errors like the
> struct one in stdlib2 or the r/rb one.
>
> SilentGhost: with statement used with open *is* about file reading :)
>
> Raymond: I agree about try/finally. I agree with Alexander about
> collections.rst.
>
Éric,
It turned out to be a non-issue, never mind.
|
msg122007 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-11-21 22:44 |
Éric, please apply most of these.
In the atexit patch, the first change is wrong. Change it to a try/except/finally or skip it altogether.
In the collections patch, only include the change for the tail example; the other two should remain unchanged.
Skip the difflib patch entirely. It unnecessarily makes the example confusing and hard to follow.
Change the cmd patch to:
+ with open(arg) as f:
+ self.cmdqueue.extend(f.read().splitlines())
|
msg122067 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2010-11-22 02:21 |
Raymond: I made a single diff for easier review. I’m not 100% sure I should change 'r' to 'rb' in logging: It’s unrelated to with, and the rest of the file has not been checked for similar errors. I prefer to do commits that don’t mix things.
Eli:
> which whitespace change do you refer to. I changed to 4-spaces
> indentation in the code sample to conform to PEP-8. Shouldn't I have?
PEP 8 only applies to Python code. http://docs.python.org/dev/documenting/style applies to the docs.
|
msg122089 - (view) |
Author: Eli Bendersky (eli.bendersky) * |
Date: 2010-11-22 04:21 |
Éric,
Yes, in a consequent patch I fixed this - kept the formatted code indented at 3 spaces, while adhering to PEP-8 internally. If there's anything else, let me know.
|
msg122095 - (view) |
Author: Raymond Hettinger (rhettinger) * |
Date: 2010-11-22 06:41 |
> I’m not 100% sure I should change 'r' to 'rb' in logging:
> It’s unrelated to with, and the rest of the file
> has not been checked for similar errors.
Good catch. This should only be a with-statement transformation. I caught other accidental semantic changes, so be continue to exercise care.
|
msg124363 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2010-12-19 17:43 |
following re-organization of the logging docs, I'm attaching updated patch.
|
msg124367 - (view) |
Author: Terry J. Reedy (terry.reedy) * |
Date: 2010-12-19 23:19 |
Vinay, you should look at the logging-cookbook patch.
|
msg124384 - (view) |
Author: Vinay Sajip (vinay.sajip) * |
Date: 2010-12-20 09:35 |
I've already made the change, Terry, just holding off committing it because Georg has frozen the py3k branch until 3.2b2 is released.
There are a few other changes I'm making, will commit these soon after 3.2b2 is released.
|
msg130580 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-03-11 15:53 |
> Perhaps something should also be added to the doc style guide (along
> with using 'attributes' instead of 'members').
Terry, could you open another report for that, or maybe just commit the change directly?
|
msg130589 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2011-03-11 16:55 |
New changeset c3d28c84a372 by Éric Araujo in branch 'default':
Use with statement where it improves the documentation (closes #10461)
http://hg.python.org/cpython/rev/c3d28c84a372
|
msg130593 - (view) |
Author: SilentGhost (SilentGhost) * |
Date: 2011-03-11 17:26 |
Éric, I'm not sure about the first atexit.rst hunk. I thought the purpose of that code was to except IOError when no file is found. I'm not entirely sure why in the simplest case infile.read() would raise IOError. I'd think that eli's patch (file19667) would suit better.
|
msg130678 - (view) |
Author: Roundup Robot (python-dev) |
Date: 2011-03-12 14:56 |
New changeset cebaf1bdaf78 by Éric Araujo in branch 'default':
Fix example in atexit doc: Both open and read could raise the IOError (#10461 follow-up).
http://hg.python.org/cpython/rev/cebaf1bdaf78
|
msg130679 - (view) |
Author: Éric Araujo (eric.araujo) * |
Date: 2011-03-12 14:58 |
Thanks for the catch! I was so used to the idiom where opening the file doesn’t fail but you protect the read or write that I didn’t think about the issue here.
|
|
Date |
User |
Action |
Args |
2022-04-11 14:57:09 | admin | set | github: 54670 |
2011-03-12 14:58:19 | eric.araujo | set | status: open -> closed
messages:
+ msg130679 nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python, python-dev stage: commit review -> resolved |
2011-03-12 14:56:30 | python-dev | set | status: pending -> open
messages:
+ msg130678 nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python, python-dev |
2011-03-11 17:26:40 | SilentGhost | set | status: closed -> pending nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python, python-dev messages:
+ msg130593
|
2011-03-11 16:55:22 | python-dev | set | status: open -> closed
nosy:
+ python-dev messages:
+ msg130589
resolution: accepted -> fixed |
2011-03-11 15:53:23 | eric.araujo | set | nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python messages:
+ msg130580 |
2011-02-04 23:50:35 | eric.araujo | link | issue11125 superseder |
2010-12-20 09:35:23 | vinay.sajip | set | nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python messages:
+ msg124384 |
2010-12-19 23:19:07 | terry.reedy | set | nosy:
georg.brandl, rhettinger, terry.reedy, vinay.sajip, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python messages:
+ msg124367 |
2010-12-19 17:43:21 | SilentGhost | set | files:
+ logging-cookbook.rst.diff nosy:
+ vinay.sajip messages:
+ msg124363
|
2010-12-19 16:47:54 | SilentGhost | set | files:
- logging.rst.diff nosy:
georg.brandl, rhettinger, terry.reedy, belopolsky, ezio.melotti, eric.araujo, eli.bendersky, SilentGhost, docs@python |
2010-11-22 06:41:45 | rhettinger | set | messages:
+ msg122095 |
2010-11-22 04:21:04 | eli.bendersky | set | messages:
+ msg122089 |
2010-11-22 02:21:37 | eric.araujo | set | files:
+ with-in-docs-1.diff
messages:
+ msg122067 |
2010-11-21 22:49:40 | eric.araujo | set | files:
- unnamed |
2010-11-21 22:44:53 | rhettinger | set | assignee: rhettinger -> eric.araujo resolution: accepted messages:
+ msg122007 stage: needs patch -> commit review |
2010-11-20 19:20:45 | eli.bendersky | set | files:
+ unnamed
messages:
+ msg121730 |
2010-11-20 18:44:35 | eric.araujo | set | messages:
+ msg121713 |
2010-11-20 18:43:14 | belopolsky | set | messages:
- msg121711 |
2010-11-20 18:32:26 | belopolsky | set | messages:
+ msg121711 |
2010-11-20 18:31:22 | rhettinger | set | assignee: docs@python -> rhettinger messages:
+ msg121710 |
2010-11-20 18:28:54 | rhettinger | set | messages:
+ msg121708 |
2010-11-20 18:24:47 | SilentGhost | set | messages:
+ msg121706 |
2010-11-20 18:21:33 | rhettinger | set | nosy:
+ rhettinger messages:
+ msg121703
|
2010-11-20 14:32:17 | SilentGhost | set | files:
- logging.rst.diff |
2010-11-20 14:32:00 | SilentGhost | set | files:
+ logging.rst.diff
messages:
+ msg121654 |
2010-11-20 14:31:56 | SilentGhost | set | files:
+ logging.rst.diff
messages:
+ msg121653 |
2010-11-20 14:08:15 | eli.bendersky | set | messages:
+ msg121648 |
2010-11-20 14:05:17 | eli.bendersky | set | files:
- issue10461.1.diff |
2010-11-20 14:05:13 | eli.bendersky | set | files:
+ issue10461.stdlib2.rst.diff |
2010-11-20 14:05:02 | eli.bendersky | set | files:
+ issue10461.atexit.rst.diff
messages:
+ msg121647 |
2010-11-20 13:59:20 | SilentGhost | set | files:
+ collections.rst.diff
messages:
+ msg121644 |
2010-11-20 13:54:14 | eli.bendersky | set | messages:
+ msg121643 |
2010-11-20 13:42:40 | SilentGhost | set | files:
+ difflib.rst.diff
messages:
+ msg121639 |
2010-11-20 13:27:35 | SilentGhost | set | files:
+ cmd.rst.diff nosy:
+ SilentGhost messages:
+ msg121638
|
2010-11-20 13:25:29 | eli.bendersky | set | messages:
+ msg121637 |
2010-11-20 13:21:23 | georg.brandl | set | nosy:
+ georg.brandl messages:
+ msg121635
|
2010-11-20 13:19:23 | eric.araujo | set | messages:
+ msg121634 |
2010-11-20 13:11:19 | eli.bendersky | set | files:
- atexit.rst |
2010-11-20 13:11:15 | eli.bendersky | set | files:
+ issue10461.1.diff keywords:
+ patch |
2010-11-20 13:10:16 | eli.bendersky | set | files:
+ atexit.rst
messages:
+ msg121630 |
2010-11-20 12:56:05 | eric.araujo | set | messages:
+ msg121627 |
2010-11-20 12:38:41 | eli.bendersky | set | nosy:
+ eli.bendersky messages:
+ msg121625
|
2010-11-20 12:36:40 | eric.araujo | set | messages:
+ msg121622 |
2010-11-19 20:13:10 | ezio.melotti | set | nosy:
+ ezio.melotti
|
2010-11-19 20:12:34 | terry.reedy | set | nosy:
+ terry.reedy messages:
+ msg121565
|
2010-11-19 19:10:31 | belopolsky | set | nosy:
+ belopolsky messages:
+ msg121559
|
2010-11-19 16:04:59 | eric.araujo | create | |