[issue21259] replace "except: pass" by "except Exception: pass"

[Raymond Hettinger]

Raymond Hettinger added the comment:

> I presume you do not mind if I reclose this.

Thanks, please do reclose this.

--

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace "except: pass" by "except Exception: pass"

[Martin Panter]

Changes by Martin Panter :


--
superseder:  -> Fix bare excepts in various places in std lib

___
Python tracker 

___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Serhiy Storchaka]

Serhiy Storchaka added the comment:

Just committed smaller patch in issue16261 and closed the issue.

Not all changes in the patch in this issue look innocent and correct.

--
nosy: +serhiy.storchaka

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Terry J. Reedy]

Terry J. Reedy added the comment:

A couple of years ago, when I pushed 'except: pass', I was told in post-review 
that grandfathered bad code is no excuse for more bad code and that I should be 
explicit, including if I actually meant except BaseException:, which in this 
case I did.  No other developer said otherwise.  I took the above to be the 
general consensus. I agree with it, one reason being that bare excepts are 
speed bumps when reading someone else's code.

Victor generic change except: pass to except Exception: pass
This is not correct without case-by-case examination.

#16261 had 2 patches.  The patch for doc examples changed 3 'except:'s to 
'except Exception:'  I believe these are correct, or correct enough.  They all 
need to *not* catch KeyboardInterrupt.  The patch for lib code never changed to 
Exception, but something tighter.

The patch committed for #16261 patched 7 files (down from the original proposed 
11).  Even that took a couple of years to get a second review.  I think further 
followup patches should probably change even fewer files and be attached to 
new, narrowly focused issues.

Raymond, since you closed this once, and since no new patch has been submitted, 
I presume you do not mind if I reclose this.

--
resolution:  - duplicate
stage:  - resolved
status: open - closed
type:  - enhancement

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Terry J. Reedy]

Terry J. Reedy added the comment:

This is definitely a duplicate of #16261 and should be closed at such.

On that issue, in msg202448, I explained that #15313 is about except: in 
idlelib and idlelib must be patched separately for the reason Raymond repeated 
(point 2). The other reason (point 1) is to keep different versions the same as 
much as possible.

I agree that bare excepts should eventually be replaced -- but on a case by 
case basis to the right specific exception. A bare except is easy to grep for. 
Once converted to something else, it is no longer visible as needing attention.

--
nosy: +terry.reedy

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Berker Peksag]

Berker Peksag added the comment:

This looks like a duplicate of issue 16261.

--
nosy: +berker.peksag

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Stéphane Wirtel]

New submission from Stéphane Wirtel:

Hi guys,

Via this issue, I will attach a patch where I replace all the except: pass in 
the source code by a clear except Exception: pass.

--
components: Library (Lib)
messages: 216524
nosy: matrixise
priority: normal
severity: normal
status: open
title: replace except: pass by except Exception: pass
versions: Python 3.5

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Stéphane Wirtel]

Stéphane Wirtel added the comment:

I attach the patch. Need review.

Thanks

--
keywords: +patch
nosy: +haypo
Added file: 
http://bugs.python.org/file34918/issue21259-replace_except_pass.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Antoine Pitrou]

Antoine Pitrou added the comment:

You shouldn't change the grammar the tests, nor the pybench source.
Also, in some places it would probably be interesting to narrow down the 
exception type more.

--
nosy: +pitrou

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Stéphane Wirtel]

Stéphane Wirtel added the comment:

I disabled the patch for pybench and the grammar.

Thanks for your review.

--
Added file: 
http://bugs.python.org/file34919/issue21259-replace_except_pass-2.patch

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Raymond Hettinger]

Raymond Hettinger added the comment:

FYI, the two are not equivalent.   A bare except is equivalent to 
BaseException.  Even then, the new code would be slower than the original, so 
I'm not sure this change should be made at all (it makes the code more verbose, 
slower, and risks introducing a semantic change).

Also, Guido traditionally tries to discourage across the board changes like 
this.  Instead, we aim for holistic refactoring where code improvements are 
being made one module at a time by someone who deeply groks the code and is 
thinking through all the changes.  The problem with across-the-board edits is 
that they tend to be very shallow change thing x to thing y without looking 
at what the surrounding code is doing or whether the code ever made sense in 
the first place.

Another part of the risk of semantic change is that we likely do not have 
specific tests for Exception vs BaseException so it would be easy to make a 
mistake and not have the test suite catch it.

--
nosy: +rhettinger

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Raymond Hettinger]

Raymond Hettinger added the comment:

After more consideration, I'm going to reject the patch because it doesn't 
really make the code better and may make it worse (changing semantics, slower, 
etc).   Any such changes should be done holistically as part of a deeper 
refactoring on an individual module.  

I appreciate the patch effort but would like to avoid code churn for nearly 
zero benefit.

--
assignee:  - rhettinger
resolution:  - rejected
status: open - closed

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[STINNER Victor]

STINNER Victor added the comment:

 FYI, the two are not equivalent.

I don't get your point, the purpose of the change is to get ride of except: 
pass which is *bad*.

 Even then, the new code would be slower than the original,

I don't understand why you are talking about performances here. Ignore 
SystemExit and KeyboardInterrupt is a huge bug, performances don't matter here.

I don't want to benchmark, but I expect that performances are exactly the same 
if no exception is raised.

Please don't close the issue, Stéphane is fixing real bug.

I agree that it would be better to split the large patch is shorter parts. Or 
all changes replacing except: pass should be grouped into the same patch.

Replacing except: code; raise with except Exception: code; raise is 
wrong.

--
resolution: rejected - 
status: closed - open

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Raymond Hettinger]

Raymond Hettinger added the comment:

 Please don't close the issue, Stéphane is fixing real bug.

He appears to be doing a blanket search and replace without adding tests, 
without evaluating each change to see if makes sense, and without consulting 
the original author of each affected piece of code.

We have never received a bug report on any one of these.  If you think any one 
of them is an actually bug, it should be carefully considered one at a time, 
evaluating why the bare except was put there in the first place.  

Guido pushes for holistic refactoring for a reason.  It is too easy to 
introduce bugs into stable code by these kind of wholesale changes.

If you (Victor) want to individually study each one to make sure it is the 
right thing to do, I would place more faith in the patch.  But as it stands 
now, reviewing the patch for correctness will take substantially more care and 
thought than it took to create it in the first place.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[STINNER Victor]

STINNER Victor added the comment:

I reviewed  issue21259-replace_except_pass-2.patch. Please split this huge 
patch into smaller patches:

1) remove dummy except: raise
2) use os.unlink()
3) replace except: pass with more precise exceptions like except OSError: 
= please write multiple small patches for this part
4) generic change except: pass to except Exception: pass

You might open a new issue for these 4 kind of patches (4 issues).

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[STINNER Victor]

STINNER Victor added the comment:

 3) replace except: pass with more precise exceptions like except OSError: 
 = please write multiple small patches for this part

You may even open a new issue just for except: pass = except OSError: pass

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[STINNER Victor]

STINNER Victor added the comment:

@Raymond: To give you more context, Stéphane is sprinting at Pycon on Pycon. I 
suggested him to fix all bare except: pass. His first patch is wrong, but 
ignoring any exception is even worse.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[STINNER Victor]

STINNER Victor added the comment:

 If you (Victor) want to individually study each one to make sure it is the 
 right thing to do, I would place more faith in the patch. But as it stands 
 now, reviewing the patch for correctness will take substantially more care 
 and thought than it took to create it in the first place.

Agreed. I would like to help Stéphane to fix these issues. I agree
that the huge patch must be splitted, I just explained how the patch
should be splitted and how Stéphane can provide more useful patches.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Raymond Hettinger]

Raymond Hettinger added the comment:

Several areas for attention:
* Changes to the test suite probably should not be made
  The user doesn't benefit in any way and you risk
  unintentionally breaking the test suite in a way that
  isn't obvious (we have no tests for the tests themselves
  so changing tests is like refactoring without a safety net).
* Some of the exception handling in IDLE needs to catch
  all exceptions (i.e. SystemExit and KeyboardInterrupt
  are supposed to display tracebacks and not terminate
  IDLE itself).
* Changing the exceptions in threading.py is worrisome.
* The Pdb debugger may have legitimate reasons to catch
  all exceptions (like IDLE, we want don't want to 
  terminate the debugger itself).

In other words, many of the proposed changes should not
be made.  

For the rest, be careful to not change semantics
unintentionally and consider adding tests if you think
there is a real bug.   In cases where there is doubt
about the right thing to do, consider assigning the
original author or maintainer of the code.

--

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com

[issue21259] replace except: pass by except Exception: pass

[Martin Panter]

Martin Panter added the comment:

If you do go ahead and add “except Exception” clauses, maybe look around at 
what other exceptions are being handled. The other day I stumbled across this 
in “tkinter.font” module:

try:
...
except (KeyboardInterrupt, SystemExit):
raise
except Exception:
pass

It would have been simpler (and semantically equivalent) to write

try:
...
except Exception:
pass

--
nosy: +vadmium

___
Python tracker rep...@bugs.python.org
http://bugs.python.org/issue21259
___
___
Python-bugs-list mailing list
Unsubscribe: 
https://mail.python.org/mailman/options/python-bugs-list/archive%40mail-archive.com