Patchwork [2,of,9,py3,v2] phases: explicitly evaluate list returned by map

login
register
mail settings
Submitter Augie Fackler
Date March 12, 2017, 5:38 p.m.
Message ID <3366d024d1d7284d4960.1489340296@augie-macbookair2.roam.corp.google.com>
Download mbox | patch
Permalink /patch/19183/
State Superseded
Headers show

Comments

Augie Fackler - March 12, 2017, 5:38 p.m.
# HG changeset patch
# User Augie Fackler <augie@google.com>
# Date 1489283600 18000
#      Sat Mar 11 20:53:20 2017 -0500
# Node ID 3366d024d1d7284d49609f6ce4e3e8a329fc8808
# Parent  66618e51771e519582cbe0fe7cd5e116d9cd87ad
phases: explicitly evaluate list returned by map

On Python 3 map() returns a generator, which bool()s to true even if
it had an empty input set. Work around this by using list() on the
map() result.
via Mercurial-devel - March 12, 2017, 5:50 p.m.
On Sun, Mar 12, 2017 at 10:38 AM, Augie Fackler <raf@durin42.com> wrote:
> # HG changeset patch
> # User Augie Fackler <augie@google.com>
> # Date 1489283600 18000
> #      Sat Mar 11 20:53:20 2017 -0500
> # Node ID 3366d024d1d7284d49609f6ce4e3e8a329fc8808
> # Parent  66618e51771e519582cbe0fe7cd5e116d9cd87ad
> phases: explicitly evaluate list returned by map
>
> On Python 3 map() returns a generator, which bool()s to true even if
> it had an empty input set. Work around this by using list() on the
> map() result.
>
> diff --git a/mercurial/phases.py b/mercurial/phases.py
> --- a/mercurial/phases.py
> +++ b/mercurial/phases.py
> @@ -213,7 +213,7 @@ class phasecache(object):
>          self._phaserevs = revs
>          self._populatephaseroots(repo)
>          for phase in trackedphases:
> -            roots = map(repo.changelog.rev, self.phaseroots[phase])
> +            roots = list(map(repo.changelog.rev, self.phaseroots[phase]))

I've heard people say that the pythonic way is to use a list
comprehension. Possibly with repo.changelog.rev extracted to a
variable for speed. Correct?

>              if roots:
>                  for rev in roots:
>                      revs[rev] = phase
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
Augie Fackler - March 12, 2017, 5:58 p.m.
> On Mar 12, 2017, at 10:50, Martin von Zweigbergk <martinvonz@google.com> wrote:
> 
> On Sun, Mar 12, 2017 at 10:38 AM, Augie Fackler <raf@durin42.com> wrote:
>> # HG changeset patch
>> # User Augie Fackler <augie@google.com>
>> # Date 1489283600 18000
>> #      Sat Mar 11 20:53:20 2017 -0500
>> # Node ID 3366d024d1d7284d49609f6ce4e3e8a329fc8808
>> # Parent  66618e51771e519582cbe0fe7cd5e116d9cd87ad
>> phases: explicitly evaluate list returned by map
>> 
>> On Python 3 map() returns a generator, which bool()s to true even if
>> it had an empty input set. Work around this by using list() on the
>> map() result.
>> 
>> diff --git a/mercurial/phases.py b/mercurial/phases.py
>> --- a/mercurial/phases.py
>> +++ b/mercurial/phases.py
>> @@ -213,7 +213,7 @@ class phasecache(object):
>>         self._phaserevs = revs
>>         self._populatephaseroots(repo)
>>         for phase in trackedphases:
>> -            roots = map(repo.changelog.rev, self.phaseroots[phase])
>> +            roots = list(map(repo.changelog.rev, self.phaseroots[phase]))
> 
> I've heard people say that the pythonic way is to use a list
> comprehension. Possibly with repo.changelog.rev extracted to a
> variable for speed. Correct?

My recollection is that map() is faster if you're only calling a function over an entire list of things. Sigh.

> 
>>             if roots:
>>                 for rev in roots:
>>                     revs[rev] = phase
>> _______________________________________________
>> Mercurial-devel mailing list
>> Mercurial-devel@mercurial-scm.org
>> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/phases.py b/mercurial/phases.py
--- a/mercurial/phases.py
+++ b/mercurial/phases.py
@@ -213,7 +213,7 @@  class phasecache(object):
         self._phaserevs = revs
         self._populatephaseroots(repo)
         for phase in trackedphases:
-            roots = map(repo.changelog.rev, self.phaseroots[phase])
+            roots = list(map(repo.changelog.rev, self.phaseroots[phase]))
             if roots:
                 for rev in roots:
                     revs[rev] = phase