Repository navigation
Sum() doc and behavior mismatch #51696
Description
| BPO | 7447 |
|---|---|
| Nosy | @birkenfeld, @rhettinger, @terryjreedy, @abalkin, @ezio-melotti, @ambv |
| Files |
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 = 'https://github.com/rhettinger'
closed_at = <Date 2010-10-31.21:30:38.750>
created_at = <Date 2009-12-06.00:50:38.907>
labels = ['docs']
title = 'Sum() doc and behavior mismatch'
updated_at = <Date 2010-10-31.21:30:38.748>
user = 'https://github.com/terryjreedy'bugs.python.org fields:
activity = <Date 2010-10-31.21:30:38.748>
actor = 'rhettinger'
assignee = 'rhettinger'
closed = True
closed_date = <Date 2010-10-31.21:30:38.750>
closer = 'rhettinger'
components = ['Documentation']
creation = <Date 2009-12-06.00:50:38.907>
creator = 'terry.reedy'
dependencies = []
files = ['17390', '18199', '18203', '18258']
hgrepos = []
issue_num = 7447
keywords = ['patch']
message_count = 21.0
messages = ['96013', '96087', '106014', '111555', '111561', '111562', '111567', '111568', '111576', '111579', '111663', '111743', '111748', '111835', '111939', '111982', '111983', '112004', '112007', '112009', '120088']
nosy_count = 8.0
nosy_names = ['georg.brandl', 'rhettinger', 'terry.reedy', 'belopolsky', 'ezio.melotti', 'docs@python', 'lvogt', 'lukasz.langa']
pr_nums = []
priority = 'low'
resolution = 'fixed'
stage = 'commit review'
status = 'closed'
superseder = None
type = None
url = 'https://bugs.python.org/issue7447'
versions = ['Python 3.1', 'Python 2.7', 'Python 3.2']
Activity
terryjreedy commentedon Dec 6, 2009
"sum(iterable[, start])
Sums start and the items of an iterable from left to right and returns
the total. start defaults to 0. The iterable‘s items are normally
numbers, and are not allowed to be strings."
The last sentence is not currently true (3.1, assume also others).
It is the start value that cannot be a string.
I am filing this as a doc issue as the easiest fix for the discrepancy
between doc and behavior. But the fix could be a behavior change, though
certain to be controversial. Given that, my suggested revision is:
"The iterable‘s items are normally numbers. The start value is not
allowed to be a string."
I think this fits the followup sentence: "The fast, correct way to
concatenate a sequence of strings..."
ezio-melotti commentedon Dec 7, 2009
There are also a couple more things that could be improved in the
documentation of sum():
particular example, it's not always true for the general case (in the
py3 doc it has been removed already).
See also 2 and the other messages in that thread.
lvogt commentedon May 18, 2010
I changed the documentation regarding string not allowed as start argument and performance
I included the list concatenation with itertools.chain from
http://groups.google.com/group/comp.lang.python/msg/33e764d0ac41826a
patch is based on revision 81300 in py3k branch.
abalkin commentedon Jul 25, 2010
A nitpick:
"lol" is a very well-known acronym and "list of lists" is not the expansion that first comes to mind.
terryjreedy commentedon Jul 25, 2010
Leonard, that you for the patch, in particular, the list of hints. I think the first is good but overly specific. It also has an extra '.itertools' in the path. Now:
'''
+ - To concatenate a list of lists ``lol`` use ``list(itertools.chain.from_iterable(lol))``
+ (see :func:`from_iterable <itertools.itertools.chain.from_iterable>`).
'''
How about instead"
'''
+ - To concatenate an iterable of iterables, such as a list of lists,
+ see :func:`from_iterable <itertools.chain.from_iterable>`
+ and call the appropriate constructor on the result, such as :class: `list`.
'''
?
birkenfeld commentedon Jul 25, 2010
Note that using
~itertools.chain.from_iterableis equivalent tofrom_iterable <itertools.chain.from_iterable>and saves a repetition.terryjreedy commentedon Jul 25, 2010
In order to have a chance at 2.6.6rc, due out in a week, I hand-edited the patch, incorporating Georg's suggestion, and uploaded. I think it is ready to commit.
Since this is not a critical doc fix, I am not marking it a blocker, but I still think it would be good to get it in.
Raymond, since you have not posted anything, I am assuming that your main interest was that the code not be changed, at least not without your notice. Since this is staying a doc issue, I am re-assigning.
abalkin commentedon Jul 25, 2010
FWIW, I like the new patch better, but still have a few nitpicks:
Starting a sentence with an argument name is a bit awkward because that makes a sentence that starts with a lower case letter.
There is an extra space in :class:
list.It would make more boring prose, but I would prefer to see similar language used in each alternative. I.e.
Why "an *iterable* of iterables" but "a *sequence* of strings"?
I am not sure it is correct to say "to concatenate an iterable", I think it should be "to concatenate items from an iterable."
terryjreedy commentedon Jul 25, 2010
patch3 incorporates all your suggestions.
I think the hints are clear enough; anyone with more questions should follow the link to the respective function doc.
1 remaining item
lvogt commentedon Jul 26, 2010
Thank you.
I think the specific list of list example is better for the sum documentation because lists support the + operator. I don't think
that someone would consider using sum for chaining arbitrary iterables.
What about a concise "To concatenate lists use itertools.chain.from_iterable." and letting the "apply list constructor" part as an exercise to the reader?
I did the explicit reference to itertools.itertools.chain.from_iterable since the default or the explicit link to itertools.chain.from_iterable did not produce a correct link in the html docs.
I applied patch3 and ran make html; there is no html link to from_iterable produced.
lvogt commentedon Jul 27, 2010
another patch:
still using itertools.itertools.chain.from_iterable as mentioned in previous message. I missed georgs use of ~ in the link, but i think its clearer to mention the full name of the function anyway.
ezio-melotti commentedon Jul 27, 2010
You could also use start=0 in the signature.
terryjreedy commentedon Jul 28, 2010
I am puzzled that the fake name 'itertools.itertools.chain.from_iterable' works better than the real name 'itertools.chain.from_iterable'. Some bug in the tool chain?
birkenfeld commentedon Jul 29, 2010
from_iterable() was marked up wrongly in the itertools docs; this is fixed now in r83230.
lvogt commentedon Jul 29, 2010
Thank you Georg, I updated the patch
Ezio, wouldn't start=0 in the signature imply that sum accepted a keyword argument? I read "Documenting Python" (4.3) but am not sure about the distinction of default values or keyword arbuments.
ambv commentedon Jul 29, 2010
The oldest interfaces tend to have quirks like that. Similar issue: bpo-9379. Won't be changed as well. I'm thinking of a better docstring though.
terryjreedy commentedon Jul 29, 2010
For Python functions, all arguments can be keyword arguments. For many builtins, none can be (and maybe there should be a note to that effect at the top of the built-in functions chapter). 'Param = default' merely means that the parameter has a default argument and it is therefore optional to provide one. It says nothing about whether the argument can be identified by the parameter name or only by position.
ambv commentedon Jul 29, 2010
+1
Many Python users don't really grasp how builtins are different from regular functions. A note of this kind would be most helpful. Should I provide a patch for Doc/ ?
terryjreedy commentedon Jul 29, 2010
Please do. This issue comes up often enough on python-list. Make it a separate doc issue, which will be auto-assigned to docs@python and add me as nosy.
rhettinger commentedon Oct 31, 2010
Fixed in r86066, r86068 and r86069.