classification
Title: Idlelib: update to "with open ... except OSError" (in 2.7, leave IOError)
Type: behavior Stage: resolved
Components: IDLE Versions: Python 3.3, Python 3.4, Python 2.7
process
Status: closed Resolution: fixed
Dependencies: Superseder:
Assigned To: terry.reedy Nosy List: Todd.Rovito, python-dev, roger.serwy, serhiy.storchaka, terry.reedy
Priority: normal Keywords: needs review, patch

Created on 2013-06-06 17:02 by terry.reedy, last changed 2013-08-04 19:42 by terry.reedy. This issue is now closed.

Files
File name Uploaded Description Edit
idle33.io2os.diff terry.reedy, 2013-06-06 17:02
grep_it.diff terry.reedy, 2013-06-06 18:30
idle33.io2os.diff terry.reedy, 2013-06-06 20:21 non-git version review
grep_it.diff terry.reedy, 2013-06-06 20:24 (3.4) non-git version review
idle_with_open.patch serhiy.storchaka, 2013-08-04 08:07 review
idle_with_open.patch serhiy.storchaka, 2013-08-04 08:08 review
idle_with_open-27.patch terry.reedy, 2013-08-04 19:42 review
Messages (11)
msg190718 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-06-06 17:02
This issue is about uniformly updating the old idiom

try:
  f = open('file')
  <use f)
  f.close()  # omitted at least in GrepDialog
except IOError as msg:
  <ignore or display message>

to the current idiom

try:
  with open('file') as f:
    <use f
except OSError as msg:  # still use IOError in 2.7
    <ignore or display message>

#16715 changed 'IOError' to 'OSError' everywhere in Lib/* for 3.4 only.

I ran into this issue (again) because GrepDialog.Grepdialog.grep_it uses open without close. Consequently, with debug builds, 'Find in files' raises a ResourceWarning for each file opened. Because of the 3.4-only change, there will be a merge conflict.

To avoid this, now and in the future, I plan to backport the idlelib subset of the Svetlov's patch to 3.3 (it applied with only one fix).

Currently, all 3.x patches are being applied to both 3.3 and 3.4 and any 3.3 patch with 'IOError' will have a merge conflict. Any 3.4 patch with 'OSError' will not apply to 3.3. There is no longer an issue with breaking 3.2 to 3.3 merges. This change will break existing 3.3 patches with 'IOError', but they would break anyway on the merge.
msg190720 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-06-06 18:30
Here is the 'OS' version of the fix and update to grep_it. Change 'OS' to 'IO' on line 17 to apply to current 3.3, without the io2os patch.  Besides using 'with', it iterates the files directly instead of iterating blocks of readlines. The two changes make the code simpler and cleaner. The changed summary looks better to me, besides avoiding the hit/hits problem.

As can be seen in the io2os patch, there are other opens to be updated in another patch.

OutputWindow.OutputWindow._file_line_helper has this one:
                    f = open(filename, "r")
                    f.close()
which would become:
                    with open(filename, "r"): pass

This tests whether the filename extracted from a traceback or grep report is correct (can be opened). (Each of several patterns are tried until one works.) The opened file is discarded because the filename is returned to be passes to editwin's flist.open. I believe this could be replaced by an os.stat() call.

Backportint to 2.7 is the subject of #18152
msg190723 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-06 18:49
Please regenerate your patches without --git for review. Rietveld doesn't like git-style patches.
msg190726 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-06-06 20:21
Serhiy: I don't care about patch style but I read somewhere that I 'should' set the hg options file to automatically produce git style diff. However, since the hg doc says that this setting does not affect pushes, I don't know why, and I cannot find where I read that. Is it obsolete? Are you suggesting that all patches should never be git style?

I turned git off and will leave it off until someone complains.
msg190730 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-06-06 20:53
Thank you. AFAIK you need git style diff only when moving/renaming files.
msg190793 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-06-08 04:38
New changeset 2fe64ce5da05 by Terry Jan Reedy in branch '3.3':
#18151, part 1: Backport idlelilb portion of Andrew Svetlov's 3.4 patch
http://hg.python.org/cpython/rev/2fe64ce5da05

New changeset 0be613638523 by Terry Jan Reedy in branch 'default':
#18151 null merge with 3.3.
http://hg.python.org/cpython/rev/0be613638523
msg191677 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-06-22 22:38
New changeset 3c0e8e910125 by Terry Jan Reedy in branch '2.7':
#18151, part 2: Silence debug build resource warning for each file opened by
http://hg.python.org/cpython/rev/3c0e8e910125

New changeset c541073173bb by Terry Jan Reedy in branch '3.3':
#18151, part 2: Silence debug build resource warning for each file opened by
http://hg.python.org/cpython/rev/c541073173bb

New changeset 1d67c0893719 by Terry Jan Reedy in branch 'default':
#18151 Merge from 3.3
http://hg.python.org/cpython/rev/1d67c0893719
msg194331 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-04 08:07
Here is a patch which replace the "open ... close" idiom to the "with open" idiom in IDLE.
msg194332 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2013-08-04 08:08
Here is a patch which replace the "open ... close" idiom to the "with open" idiom in IDLE.
msg194414 - (view) Author: Roundup Robot (python-dev) (Python triager) Date: 2013-08-04 19:40
New changeset e450e85e2075 by Terry Jan Reedy in branch '3.3':
Issue #18151: Replace remaining Idle 'open...close' pairs with 'with open'.
http://hg.python.org/cpython/rev/e450e85e2075

New changeset 7f6661a90d02 by Terry Jan Reedy in branch '2.7':
Issue #18151: Replace remaining Idle 'open...close' pairs with 'with open'.
http://hg.python.org/cpython/rev/7f6661a90d02
msg194415 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2013-08-04 19:42
Patch looked good, so I backported to 2.7 (attached, in case there are problems) and pushed.
History
Date User Action Args
2013-08-04 19:42:50terry.reedysetstatus: open -> closed
files: + idle_with_open-27.patch
messages: + msg194415

assignee: terry.reedy
resolution: fixed
stage: patch review -> resolved
2013-08-04 19:40:21python-devsetmessages: + msg194414
2013-08-04 08:08:32serhiy.storchakasetfiles: + idle_with_open.patch

messages: + msg194332
2013-08-04 08:07:20serhiy.storchakasetfiles: + idle_with_open.patch

messages: + msg194331
stage: commit review -> patch review
2013-06-22 22:38:04python-devsetmessages: + msg191677
2013-06-08 04:38:09python-devsetnosy: + python-dev
messages: + msg190793
2013-06-06 20:53:53serhiy.storchakasetmessages: + msg190730
2013-06-06 20:24:22terry.reedysetfiles: + grep_it.diff
2013-06-06 20:21:30terry.reedysetfiles: + idle33.io2os.diff

messages: + msg190726
2013-06-06 18:49:37serhiy.storchakasetnosy: + serhiy.storchaka
messages: + msg190723
2013-06-06 18:30:43terry.reedysetkeywords: + needs review
files: + grep_it.diff
messages: + msg190720
2013-06-06 17:02:32terry.reedycreate