Patchwork [06,of,10] py3: add @util.boolclass markers for boolean classes

login
register
mail settings
Submitter timeless
Date May 12, 2016, 1:23 a.m.
Message ID <d0b1768012385bb1ceac.1463016194@gcc2-power8.osuosl.org>
Download mbox | patch
Permalink /patch/15054/
State Under Review, archived
Delegated to: Yuya Nishihara
Headers show

Comments

timeless - May 12, 2016, 1:23 a.m.
# HG changeset patch
# User timeless <timeless@mozdev.org>
# Date 1463009260 0
#      Wed May 11 23:27:40 2016 +0000
# Node ID d0b1768012385bb1ceacace2f6c32ee28a29f46e
# Parent  e670b15b74f80135c28e3ac57b0580ad98b47cb9
# Available At bb://timeless/mercurial-crew
#              hg pull bb://timeless/mercurial-crew -r d0b176801238
py3: add @util.boolclass markers for boolean classes
Yuya Nishihara - May 18, 2016, 1:55 p.m.
On Thu, 12 May 2016 01:23:14 +0000, timeless wrote:
> # HG changeset patch
> # User timeless <timeless@mozdev.org>
> # Date 1463009260 0
> #      Wed May 11 23:27:40 2016 +0000
> # Node ID d0b1768012385bb1ceacace2f6c32ee28a29f46e
> # Parent  e670b15b74f80135c28e3ac57b0580ad98b47cb9
> # Available At bb://timeless/mercurial-crew
> #              hg pull bb://timeless/mercurial-crew -r d0b176801238
> py3: add @util.boolclass markers for boolean classes

> --- a/mercurial/context.py	Thu May 12 00:06:19 2016 +0000
> +++ b/mercurial/context.py	Wed May 11 23:27:40 2016 +0000
> @@ -395,6 +395,7 @@
>                   date, extra, editor)
>      return ctx
>  
> +@util.boolclass
>  class changectx(basectx):
>      """A changecontext object makes access to data related to a particular
>      changeset convenient. It represents a read-only context already present in
> @@ -707,6 +708,10 @@
>              # file is missing
>              return False
>  
> +    def __bool__(self):
> +        """Python 3 equivalent of Python 2 __nonzero__"""
> +        return self.__nonzero__()
> +
>      def __str__(self):
>          return "%s@%s" % (self.path(), self._changectx)
>  
> @@ -1180,6 +1185,10 @@
>      def __nonzero__(self):
>          return True
>  
> +    def __bool__(self):
> +        """Python 3 equivalent of Python 2 __nonzero__"""
> +        return self.__nonzero__()
> +
>      def _buildflagfunc(self):
>          # Create a fallback function for getting file flags when the
>          # filesystem doesn't support them
> @@ -1675,6 +1684,10 @@
>      def __nonzero__(self):
>          return True
>  
> +    def __bool__(self):
> +        """Python 3 equivalent of Python 2 __nonzero__"""
> +        return self.__nonzero__()

Just curious why these three don't use @util.boolclass.

Also, if __bool__ is always forwarded to __nonzero__ like these, we don't
need to define __bool__ in all sub classes.
timeless - May 31, 2016, 6:49 p.m.
Yuya Nishihara wrote:
> Just curious why these three don't use @util.boolclass.

I probably hadn't hit it in testing.

> Also, if __bool__ is always forwarded to __nonzero__ like these, we don't
> need to define __bool__ in all sub classes.

I changed my local files to that, and then I thought about it....

My implementation in e670b15b74f80135c28e3ac57b0580ad98b47cb9 would do
the wrong thing for subclasses:

$ cat x.py; python x.py
def boolclass(cls):
    """class decorator for classes with __bool__ method"""
    cls.__bool__ = cls.__nonzero__
    return cls

@boolclass
class X(object):
    def __nonzero__(self):
        return True

class Y(X):
    def __nonzero__(self):
        return False

x=X()
y=Y()
print(x.__nonzero__(),x.__bool__(),y.__nonzero__(),y.__bool__())
(True, True, False, True)

I think for now, I'll just have it do `return self.__nonzero__()`:
def boolclass(cls):
    """class decorator for classes with __bool__ method"""
    def bool_nonzero(self):
        return self.__nonzero__()
    cls.__bool__ = bool_nonzero
    return cls

I'll resubmit this pair with these things addressed...
Yuya Nishihara - June 1, 2016, 12:37 p.m.
On Tue, 31 May 2016 14:49:12 -0400, timeless wrote:
> Yuya Nishihara wrote:
> > Just curious why these three don't use @util.boolclass.  
> 
> I probably hadn't hit it in testing.
> 
> > Also, if __bool__ is always forwarded to __nonzero__ like these, we don't
> > need to define __bool__ in all sub classes.  
> 
> I changed my local files to that, and then I thought about it....
> 
> My implementation in e670b15b74f80135c28e3ac57b0580ad98b47cb9 would do
> the wrong thing for subclasses:
> 
> $ cat x.py; python x.py
> def boolclass(cls):
>     """class decorator for classes with __bool__ method"""
>     cls.__bool__ = cls.__nonzero__
>     return cls
> 
> @boolclass
> class X(object):
>     def __nonzero__(self):
>         return True
> 
> class Y(X):
>     def __nonzero__(self):
>         return False
> 
> x=X()
> y=Y()
> print(x.__nonzero__(),x.__bool__(),y.__nonzero__(),y.__bool__())
> (True, True, False, True)
> 
> I think for now, I'll just have it do `return self.__nonzero__()`:
> def boolclass(cls):
>     """class decorator for classes with __bool__ method"""
>     def bool_nonzero(self):
>         return self.__nonzero__()
>     cls.__bool__ = bool_nonzero
>     return cls

Yes. IIRC, normal functions in cls are bound to its instance, so it should
be fine to assign a function to __bool__. Another option is to (ab)use
inheritance,

  class boolclass(object):
      def __bool__(self):
          return self.__nonzero__()
  class X(boolclass):
      ...

or do everything manually with no magic,

  class X(object):
      ...
      __bool__ = __nonzero__

I don't know which is better.

Patch

diff -r e670b15b74f8 -r d0b176801238 mercurial/ancestor.py
--- a/mercurial/ancestor.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/ancestor.py	Wed May 11 23:27:40 2016 +0000
@@ -11,6 +11,7 @@ 
 import heapq
 
 from .node import nullrev
+from . import util
 
 def commonancestorsheads(pfunc, *nodes):
     """Returns a set with the heads of all common ancestors of all nodes,
@@ -257,6 +258,7 @@ 
         missing.reverse()
         return missing
 
+@util.boolclass
 class lazyancestors(object):
     def __init__(self, pfunc, revs, stoprev=0, inclusive=False):
         """Create a new object generating ancestors for the given revs. Does
diff -r e670b15b74f8 -r d0b176801238 mercurial/bundle2.py
--- a/mercurial/bundle2.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/bundle2.py	Wed May 11 23:27:40 2016 +0000
@@ -225,6 +225,7 @@ 
         return func
     return _decorator
 
+@util.boolclass
 class unbundlerecords(object):
     """keep record of what happens during and unbundle
 
diff -r e670b15b74f8 -r d0b176801238 mercurial/context.py
--- a/mercurial/context.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/context.py	Wed May 11 23:27:40 2016 +0000
@@ -395,6 +395,7 @@ 
                  date, extra, editor)
     return ctx
 
+@util.boolclass
 class changectx(basectx):
     """A changecontext object makes access to data related to a particular
     changeset convenient. It represents a read-only context already present in
@@ -707,6 +708,10 @@ 
             # file is missing
             return False
 
+    def __bool__(self):
+        """Python 3 equivalent of Python 2 __nonzero__"""
+        return self.__nonzero__()
+
     def __str__(self):
         return "%s@%s" % (self.path(), self._changectx)
 
@@ -1180,6 +1185,10 @@ 
     def __nonzero__(self):
         return True
 
+    def __bool__(self):
+        """Python 3 equivalent of Python 2 __nonzero__"""
+        return self.__nonzero__()
+
     def _buildflagfunc(self):
         # Create a fallback function for getting file flags when the
         # filesystem doesn't support them
@@ -1675,6 +1684,10 @@ 
     def __nonzero__(self):
         return True
 
+    def __bool__(self):
+        """Python 3 equivalent of Python 2 __nonzero__"""
+        return self.__nonzero__()
+
     def linkrev(self):
         # linked to self._changectx no matter if file is modified or not
         return self.rev()
diff -r e670b15b74f8 -r d0b176801238 mercurial/formatter.py
--- a/mercurial/formatter.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/formatter.py	Wed May 11 23:27:40 2016 +0000
@@ -24,6 +24,7 @@ 
 
 pickle = util.pickle
 
+@util.boolclass
 class baseformatter(object):
     def __init__(self, ui, topic, opts):
         self._ui = ui
@@ -66,6 +67,7 @@ 
         if self._item is not None:
             self._showitem()
 
+@util.boolclass
 class plainformatter(baseformatter):
     '''the default text output scheme'''
     def __init__(self, ui, topic, opts):
diff -r e670b15b74f8 -r d0b176801238 mercurial/hgweb/webutil.py
--- a/mercurial/hgweb/webutil.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/hgweb/webutil.py	Wed May 11 23:27:40 2016 +0000
@@ -58,6 +58,7 @@ 
         yield 3 * step
         step *= 10
 
+@util.boolclass
 class revnav(object):
 
     def __init__(self, repo):
diff -r e670b15b74f8 -r d0b176801238 mercurial/localrepo.py
--- a/mercurial/localrepo.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/localrepo.py	Wed May 11 23:27:40 2016 +0000
@@ -228,6 +228,7 @@ 
     def changegroupsubset(self, bases, heads, source):
         return changegroup.changegroupsubset(self._repo, bases, heads, source)
 
+@util.boolclass
 class localrepository(object):
 
     supportedformats = set(('revlogv1', 'generaldelta', 'treemanifest',
diff -r e670b15b74f8 -r d0b176801238 mercurial/revset.py
--- a/mercurial/revset.py	Thu May 12 00:06:19 2016 +0000
+++ b/mercurial/revset.py	Wed May 11 23:27:40 2016 +0000
@@ -2481,6 +2481,7 @@ 
     else:
         return repr(r)
 
+@util.boolclass
 class abstractsmartset(object):
 
     def __nonzero__(self):
@@ -2590,6 +2591,7 @@ 
             condition = util.cachefunc(condition)
         return filteredset(self, condition, condrepr)
 
+@util.boolclass
 class baseset(abstractsmartset):
     """Basic data structure that represents a revset and contains the basic
     operation that it should be able to perform.
@@ -2703,6 +2705,7 @@ 
             s = repr(l)
         return '<%s%s %s>' % (type(self).__name__, d, s)
 
+@util.boolclass
 class filteredset(abstractsmartset):
     """Duck type for baseset class which iterates lazily over the revisions in
     the subset and contains a function which tests for membership in the
@@ -2842,6 +2845,7 @@ 
         for val in it:
             yield val
 
+@util.boolclass
 class addset(abstractsmartset):
     """Represent the addition of two sets
 
@@ -3045,6 +3049,7 @@ 
         d = {None: '', False: '-', True: '+'}[self._ascending]
         return '<%s%s %r, %r>' % (type(self).__name__, d, self._r1, self._r2)
 
+@util.boolclass
 class generatorset(abstractsmartset):
     """Wrap a generator for lazy iteration
 
@@ -3219,6 +3224,7 @@ 
         d = {False: '-', True: '+'}[self._ascending]
         return '<%s%s>' % (type(self).__name__, d)
 
+@util.boolclass
 class spanset(abstractsmartset):
     """Duck type for baseset class which represents a range of revisions and
     can work lazily and without having all the range in memory