classification
Title: map and filter shouldn't support None as first argument (in Py3k only)
Type: behavior Stage:
Components: Interpreter Core Versions: Python 3.0
process
Status: closed Resolution: accepted
Dependencies: Superseder:
Assigned To: rhettinger Nosy List: belopolsky, benjamin.peterson, gvanrossum, rhettinger
Priority: normal Keywords: easy, patch

Created on 2008-02-25 04:04 by gvanrossum, last changed 2008-03-12 22:41 by belopolsky. This issue is now closed.

Files
File name Uploaded Description Edit
filter.diff belopolsky, 2008-02-25 15:52
Messages (19)
msg62967 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-02-25 04:04
There are other ways of getting the same effects now (list() or zip()
for map(None, ...)).
msg62970 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-25 04:52
In the absence of an identity function, map accepting None is useful in 
the cases like this:

converters = {..}
y = map(converters.get(c), x)

That will now have to be rewritten as

conv = converters.get(c)
if conv is None:
   y = list(x)
else:
   y = map(conv, x)

and subtle bugs will be introduced if x is used instead of list(x) in 
the None case.

Another alternative,

y = map(converters.get(c, lambda x:x), x)

will be much slower.

Take my opinion with a grain of salt because I also believe None should 
be callable with None(x) -> x and None(x,y,..) -> (x,y,..).
msg62984 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-25 15:52
Attached patch removes support for None from filter and 
itertools.ifilter.  My objections for removing that from map do not 
apply because bool function can be used instead of None in filter 
achieving similar performance and better clarity.

None support was apparently removed from map in r60206.

Remaining questions:

1. map(None,[]), filter(None,[]) and in fact map(anything,[]) still 
work.  I don't think that needs to be fixed.

2. Should None support be removed from itertools.ifilterfalse? I would 
guess yes, unless it is going away.
msg62986 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-02-25 17:03
Let me assign this to Raymond for the answering of the question about
ifilterfalse.
msg62991 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-25 19:08
Will think about ifilter() and ifilterfalse() for a day or two.  The
None argument handles the common use-case and there may be a good case
for keeping it.  If I do decide to drop it, will put in my own patch
that optimized for the case with the filter function is bool().

+1 On removing the None argument from map() and imap().  It was
hold-over from the days before zip().
msg63002 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-25 21:10
Raymond,

There must be a reason why we constantly disagree.  Do you live in
California by any chance? :-)

I am not sure if map(None, ..) fate is still up for voting given your
changes at r60206, but your own patch illustrates the problem that I
highlighted:

< it = izip(map(key, in1), count(), in2)                  # decorate
--
> keys = in1 if key is None else map(key, in1)
> it = izip(keys, count(), in2)                           # decorate

See
http://svn.python.org/view/python/branches/py3k/Lib/heapq.py?rev=60206&view=diff&r1=60206&r2=60205&p1=python/branches/py3k/Lib/heapq.py&p2=/python/branches/py3k/Lib/heapq.py

With filter, availability of bool as a substitute for None is probably
making special handling unnecessary because __nonzero__ is called on
every object anyways.

In the case of map(None,...) the optimization is more substantial and
without None is precluded by the absence of a singleton identity function.
msg63007 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-25 22:08
FWIW, I didn't disagree on filter().  Am taking your suggestion under
advisement for a couple days.  At this point, I'm leaning towards
accepting the request (although with a different version of the patch).

For map(None, ...), I happy to live with examples like the patch you
referenced. It's important that this API be as simple and clean as
possible.  It's a matter of taste.  I concur with Guido that we never
would have created map(None, ...) if zip() had existed.  The primary use
case is obsolete.  

Also, I think the motivation behind your other use case will likey be
addressed in a more general way with an identify function being added to
the operator module.  That is a more explicit and less hackish than
having a special meaning for None.  It is also more in-line with the way
functional language approach the same problem.

Will meditate on both of these for a couple days and get back.
msg63013 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-25 23:06
Raymond,

it looks like you just broke the build for me:

/Users/sasha/Work/python-svn/trunk/Modules/itertoolsmodule.c: In 
function 'ifilter_next':
/Users/sasha/Work/python-svn/trunk/Modules/itertoolsmodule.c:2058: 
error: invalid operands to binary ==
/Users/sasha/Work/python-svn/trunk/Modules/itertoolsmodule.c: In 
function 'ifilterfalse_next':
/Users/sasha/Work/python-svn/trunk/Modules/itertoolsmodule.c:2202: 
error: invalid operands to binary ==

$ svn update Modules/itertoolsmodule.c
At revision 61073.

(Mac OS 10.4)
msg63014 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-25 23:10
Alexander, please contact me directly at python at rcn dot com.

Need to figure-out why this works on my build but not yours.  There may
be an include file issue.
msg63018 - (view) Author: Benjamin Peterson (benjamin.peterson) * (Python committer) Date: 2008-02-25 23:53
It appears Facundo fixed your problem.
msg63021 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-26 01:56
What do you guys think about just making the predicate argument optional? 

   filter([-2,0,2]) --> -2, 2
   filter(pred, iterable)

One arg is the first case and two args is the second case.
msg63022 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-26 02:13
> What do you guys think about just making the predicate argument 
> optional?

You've read my mind! That what I was going to suggest if I realized that 
optional argument does not have to be the last one.

Looks like it would make sense to keep filterfalse with a similar 
change.
msg63024 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-02-26 03:03
> I think the motivation behind your other use case will likey be
> addressed in a more general way with an identify function being
> added to the operator module.

Quite true, but I thought operator.identity was dead and buried.  Can 
you update issue1673203 if some decision was made?

Note that in msg55022 I said: "If [the identity] proposal is accepted, 
it will make sense to deprecate the use of None as an identity function 
in map."  So there may be no disagreement at all.
msg63026 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-02-26 04:40
>  What do you guys think about just making the predicate argument optional?
>
>    filter([-2,0,2]) --> -2, 2
>    filter(pred, iterable)
>
>  One arg is the first case and two args is the second case.

-1. Apart from range() this is used nowhere else in Python. (Well, it
is in curses, but it's an abomination there too.) I'd rather keep
explicit None that *this*, though I really don't see what's wrong with
filter(bool, ...) -- you can even optimize for that pattern.
msg63027 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-02-26 04:59
Okay.  Will drop None in favor of bool() in ifilter and ifilterfalse. 
Also, it looks like there agreement on dropping None for map() and 
going forward with the operator.identity() patch.  Will check these in 
in the next couple of days.
msg63483 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-12 20:13
Do you guys see any merit in changing the argument order for ifilter so
that the predicate function can just be an optional argument:

  ifilter(data[, pred])

Alex Martelli successfully lobbied for groupby() to have that same
argument order.  Originally, the signature was groupby(key, iterable)
but it worked-out much better to have groupby(iterable[, key]).

Also, I like swapped arguments because it more closely parallels the
order used in an equivalent list comprehension:

   [e for e in iterable if pred(e)]
msg63484 - (view) Author: Guido van Rossum (gvanrossum) * (Python committer) Date: 2008-03-12 21:31
It would break the symmetry with map().
msg63488 - (view) Author: Raymond Hettinger (rhettinger) * (Python committer) Date: 2008-03-12 22:24
Okay, thanks.  Though, I should have also mentioned symmetries with
sorted(), min(), and max() which all take the iterable first and follow
with an optional key function.

Closing this one. The map(None, *args) feature was removed for 3.0. 
Have decided to leave None in ifilter() and ifilterfalse() since None is
the preferred way to spell a default argument and having to pass "bool"
feels awkward and clunky. Switching the argument order was shot down, so
will just leave it as-is.

Marked resolution as "accepted" since the map(None) suggestion went in.
msg63490 - (view) Author: Alexander Belopolsky (belopolsky) * (Python committer) Date: 2008-03-12 22:41
It may be too late to express my opinion, but why symmetry with map is 
so important?  There are several reasons why sequence, predicate order 
is natural for filter and function, sequence is a natural order for map.

1. In list comprehensions predicate comes last: compare [f(x) for x in 
seq] and [x for x in seq if pred(x)].

2. In English, map(f, sec) = "Map f over seq" while filter(pred, seq) = 
filter seq through pred.
History
Date User Action Args
2008-03-12 22:41:50belopolskysetmessages: + msg63490
2008-03-12 22:24:40rhettingersetstatus: open -> closed
resolution: accepted
messages: + msg63488
2008-03-12 21:31:55gvanrossumsetmessages: + msg63484
2008-03-12 20:13:45rhettingersetmessages: + msg63483
2008-02-26 04:59:58rhettingersetmessages: + msg63027
2008-02-26 04:41:00gvanrossumsetmessages: + msg63026
2008-02-26 03:03:14belopolskysetmessages: + msg63024
2008-02-26 02:13:29belopolskysetmessages: + msg63022
2008-02-26 01:56:34rhettingersetmessages: + msg63021
2008-02-25 23:53:12benjamin.petersonsetnosy: + benjamin.peterson
messages: + msg63018
2008-02-25 23:10:20rhettingersetmessages: + msg63014
2008-02-25 23:06:20belopolskysetmessages: + msg63013
2008-02-25 22:08:24rhettingersetmessages: + msg63007
2008-02-25 21:10:42belopolskysetmessages: + msg63002
2008-02-25 19:08:27rhettingersetmessages: + msg62991
2008-02-25 17:03:30gvanrossumsetassignee: rhettinger
messages: + msg62986
nosy: + rhettinger
2008-02-25 15:52:55belopolskysetfiles: + filter.diff
keywords: + patch
messages: + msg62984
2008-02-25 04:52:15belopolskysetnosy: + belopolsky
messages: + msg62970
2008-02-25 04:04:49gvanrossumsettype: behavior
components: + Interpreter Core
2008-02-25 04:04:20gvanrossumcreate