Redundant try-except block in urllib

[PashaWNN]

BPO	35447
Nosy	@vadmium, @serhiy-storchaka, @PashaWNN
PRs	bpo-35447: Removed redundant try-except block #11051 bpo-35447: Removed redundant try-except block #11052

^{Note: these values reflect the state of the issue at the time it was migrated and might not reflect the current state.}

Show more details

GitHub fields:

assignee = None
closed_at = <Date 2018-12-10.10:06:51.651>
created_at = <Date 2018-12-09.15:23:18.194>
labels = ['3.7', '3.8', 'type-feature', 'library', 'invalid']
title = 'Redundant try-except block in urllib'
updated_at = <Date 2018-12-10.10:06:51.649>
user = 'https://github.com/PashaWNN'

bugs.python.org fields:

activity = <Date 2018-12-10.10:06:51.649>
actor = 'serhiy.storchaka'
assignee = 'none'
closed = True
closed_date = <Date 2018-12-10.10:06:51.651>
closer = 'serhiy.storchaka'
components = ['Library (Lib)']
creation = <Date 2018-12-09.15:23:18.194>
creator = 'PashaWNN'
dependencies = []
files = []
hgrepos = []
issue_num = 35447
keywords = ['patch']
message_count = 5.0
messages = ['331436', '331464', '331465', '331468', '331481']
nosy_count = 3.0
nosy_names = ['martin.panter', 'serhiy.storchaka', 'PashaWNN']
pr_nums = ['11051', '11052']
priority = 'normal'
resolution = 'not a bug'
stage = 'resolved'
status = 'closed'
superseder = None
type = 'enhancement'
url = 'https://bugs.python.org/issue35447'
versions = ['Python 2.7', 'Python 3.4', 'Python 3.5', 'Python 3.6', 'Python 3.7', 'Python 3.8']

[PashaWNN]

https://github.com/python/cpython/blob/master/Lib/urllib/parse.py#L875
It's redundant to raise and then catch exception.

[vadmium]

Code in question:

try:
# non-sequence items should not work with len()
# non-empty strings will fail this
if len(query) and not isinstance(query[0], tuple):
raise TypeError
# [. . .]
except TypeError:
ty, va, tb = sys.exc_info()
raise TypeError("not a valid non-string sequence "
"or mapping object").with_traceback(tb)

It is not redundant if you want a specific error message. I think that is the point of the code, to indicate the problem to the programmer. So the message is meant to be useful. I can think of three cases:

>>> urlencode(None)  # Non-sequence
TypeError: not a valid non-string sequence or mapping object
>>> urlencode({'sized but not indexable'})
TypeError: not a valid non-string sequence or mapping object
>>> urlencode('item [0] not a tuple')
TypeError: not a valid non-string sequence or mapping object

[PashaWNN]

https://github.com/python/cpython/blob/master/Lib/urllib/parse.py#L875
It's redundant to raise and then catch exception.

I mean:
if len(query) and not isinstance(query[0], tuple):
ty, va, tb = sys.exc_info()
raise TypeError("not a valid non-string sequence "
"or mapping object").with_traceback(tb)

Would be more clean.

[vadmium]

That would not include the custom error message for the first two cases I listed. I suggest closing this.

[serhiy-storchaka]

I concur with Martin. The try-except block was added for purpose.