classification
Title: urlencode should accept iterables of pairs
Type: enhancement Stage:
Components: Library (Lib) Versions: Python 3.4
process
Status: open Resolution:
Dependencies: Superseder:
Assigned To: Nosy List: Thomas Fenzl, eric.araujo, ezio.melotti, nagisa, orsenthil, terry.reedy, thomaslee
Priority: normal Keywords: patch

Created on 2012-09-09 10:18 by nagisa, last changed 2014-04-17 18:12 by eric.araujo.

Files
File name Uploaded Description Edit
issue-15887-01.patch thomaslee, 2012-09-15 22:24 Issue 15887 WIP patch v1 review
Messages (6)
msg170094 - (view) Author: Simonas Kazlauskas (nagisa) Date: 2012-09-09 10:18
``urllib.parse.urlencode([('i', i) for i in range(1000)])`` works, but ``urlencode(('i', i) for i in range(1000))`` raises a ``TypeError: not a valid non-string sequence or mapping object``.

Allowing urlencode to accept generators would make life a bit easier.
msg170281 - (view) Author: Senthil Kumaran (orsenthil) * (Python committer) Date: 2012-09-11 09:08
This is a feature request. urlencode tries to prepare the data in the form items are submitted. It has been like form is list of key,value pairs and that's what is reflect in the current behavior. That said, I am not -1 if see if some frameworks adopting generator approach or if it cna considered useful without any confusion. I still have soem doubts, namely if it can be just key,value pairs then why have a list at all?
msg170497 - (view) Author: Terry J. Reedy (terry.reedy) * (Python committer) Date: 2012-09-14 19:11
The first sentence of the urlencode entry is a bit garbled as something is clearly missing.

"Convert a mapping object or a sequence of two-element tuples, which may either be a str or a bytes, to a “percent-encoded” string."

Mappings, duple sequences, and duples cannot be str or bytes. I presume the query argument can be a string (as stated in the docstring, but not here, though only vacuously true since only empty strings are accepted -- see below), mapping, or duple sequence, and that the phrase "which may either be a str or a bytes" refers to the unstated string option. This sentence should be fixed after deciding that the new truth should be.

The stated requirement of 'two-element tuples' is overly strict. The two loop headers
  for k, v in query:
only requires pairs, or rather iterables that produce two items, just as in two-target assignment statements.

The enhancement proposal should be the expansion of 'sequence' to 'iterable'. This is in line with the general trend in 3.x of replacing list or sequence with iterable. There is no good reason to force the creation of an temporary list. The two loops
  for k, v in query:
work perfectly well with any key-value iterable. In fact, dict (mapping) queries are now (in Python3) handled by replacing them with a key-value iterable that is not a sequence.
    if hasattr(query, "items"):
        query = query.items()
So directly passing dict.items(), for instance, should be allowed.

The only change needed is to the query argument rejection logic.

Currently, non-empty strings are rejected, even though the doc string and docs state or imply that query may be a string. They should state that only empty strings are accepted (though I do not see the point of accepting and returning an empty string, so maybe empty strings should also be disallowed). I believe the following reproduces current behavior as regards strings except for giving a more accurate error message.

if isinstance(query, (str, bytes)):
  if len(query): raise TypeError("non empty strings not allowed")
  else: return query

.with_traceback is meant for when one replaces one exception type with another or modified the tb object. The current usage in the code does neither and is a no-op and should be omitted in a patch.

The main part of the code can be wrapped with try-except to customize an error message if query is not iterable or if the items produced by iterating query are not iterables of pairs.
msg170530 - (view) Author: Thomas Lee (thomaslee) (Python committer) Date: 2012-09-15 21:32
Working on a patch for this. Should be ready for review shortly.
msg170536 - (view) Author: Thomas Lee (thomaslee) (Python committer) Date: 2012-09-15 22:24
Patch attached. Terry, you'll notice I wrap the main part of the urlencode function in a big try/except block to catch TypeErrors & ValueErrors.

I'm a bit concerned that doing this may misreport the true underlying error in the event we screw up values/types in a way that doesn't relate to the 'query' argument, but I think the code seems to do a pretty good job of ensuring that won't happen -- so that fear may be unfounded.

You'll see I reworked the documentation quite a bit. The wording was a bit odd in places, and I called out a few things that may not be obvious at a glance. Hopefully it seems like an improvement. :)
msg216720 - (view) Author: Éric Araujo (eric.araujo) * (Python committer) Date: 2014-04-17 18:12
>> "Convert a mapping object or a sequence of two-element tuples, which
>> may either be a str or a bytes, to a “percent-encoded” string."
>
> Mappings, duple sequences, and duples cannot be str or bytes.

The only thing making sense to me here is that “which” refers to “element”.  Rephrased: Convert a mapping or sequence of two-element tuples, where each mapping key or each first element in the tuples may be str or bytes.
History
Date User Action Args
2014-04-17 18:12:11eric.araujosetnosy: + eric.araujo
messages: + msg216720
2014-04-16 22:00:54Thomas Fenzlsetnosy: + Thomas Fenzl
2012-09-15 22:24:12thomasleesetfiles: + issue-15887-01.patch
keywords: + patch
messages: + msg170536
2012-09-15 21:32:47thomasleesetnosy: + thomaslee
messages: + msg170530
2012-09-15 06:03:05ezio.melottisetnosy: + ezio.melotti
2012-09-14 19:11:23terry.reedysetnosy: + terry.reedy

messages: + msg170497
title: urlencode should accept generators or two elements tuples -> urlencode should accept iterables of pairs
2012-09-11 09:08:21orsenthilsetnosy: + orsenthil
messages: + msg170281
2012-09-10 23:33:29berker.peksagsetversions: + Python 3.4, - Python 3.2
2012-09-09 10:18:56nagisacreate