classification
Title: Redundant try-except block in urllib
Type: enhancement Stage: resolved
Components: Library (Lib) Versions: Python 3.8, Python 3.7, Python 3.6, Python 3.4, Python 3.5, Python 2.7
process
Status: closed Resolution: not a bug
Dependencies: Superseder:
Assigned To: Nosy List: PashaWNN, martin.panter, serhiy.storchaka
Priority: normal Keywords: patch

Created on 2018-12-09 15:23 by PashaWNN, last changed 2018-12-10 10:06 by serhiy.storchaka. This issue is now closed.

Pull Requests
URL Status Linked Edit
PR 11051 closed PashaWNN, 2018-12-09 15:29
PR 11052 closed PashaWNN, 2018-12-09 15:35
Messages (5)
msg331436 - (view) Author: Shishmarev Pavel (PashaWNN) * Date: 2018-12-09 15:23
https://github.com/python/cpython/blob/master/Lib/urllib/parse.py#L875
It's redundant to raise and then catch exception.
msg331464 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2018-12-10 04:08
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
msg331465 - (view) Author: Shishmarev Pavel (PashaWNN) * Date: 2018-12-10 04:17
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.
msg331468 - (view) Author: Martin Panter (martin.panter) * (Python committer) Date: 2018-12-10 04:35
That would not include the custom error message for the first two cases I listed. I suggest closing this.
msg331481 - (view) Author: Serhiy Storchaka (serhiy.storchaka) * (Python committer) Date: 2018-12-10 10:06
I concur with Martin. The try-except block was added for purpose.
History
Date User Action Args
2018-12-10 10:06:51serhiy.storchakasetstatus: open -> closed

nosy: + serhiy.storchaka
messages: + msg331481

resolution: not a bug
stage: patch review -> resolved
2018-12-10 04:35:42martin.pantersetmessages: + msg331468
2018-12-10 04:17:07PashaWNNsetmessages: + msg331465
2018-12-10 04:08:09martin.pantersetnosy: + martin.panter
messages: + msg331464
2018-12-09 15:35:35PashaWNNsetpull_requests: + pull_request10288
2018-12-09 15:29:03PashaWNNsetkeywords: + patch
stage: patch review
pull_requests: + pull_request10287
2018-12-09 15:23:18PashaWNNcreate