__repr__ for a binary tree whose constructor has default arguments

One thing you can simplify is this subexpression:

repr(self.left) if self.left else None

to just:

repr(self.left)

since repr(None) == str(None) == 'None'. Just saves some typing.

Furthermore, when you are doing:

'({}, {}, {})'.format(repr(a), repr(b), repr(c))

that is equivalent to just repring the tuple:

repr((a, b, c))

Other than that, is it worth the added complexity of the replaces? It's not the end of the world that:

repr(BinaryTree(2)) == 'BinaryTree(2, None, None)'

I'd say just keep it simple:

return '{}{}'.format(
          self.__class__.__name__,
          repr((self.value, self.left, self.right)))

Sometimes it's best just to write simple code and not worry about a small amount of repetition:

def __repr__(self):
    if self.right is not None:
        fmt = '{}({value!r}, {left!r}, {right!r})'
    elif self.left is not None:
        fmt = '{}({value!r}, {left!r})'
    else:
        fmt = '{}({value!r})'
    return fmt.format(type(self).__name__, **vars(self))

I have to agree with you BinaryTree(1, None, None) is not very nice to look at.

First off in def __init__, left = None should be left=None to comply fully with PEP8.

Don't use spaces around the = sign when used to indicate a keyword argument or a default parameter value.

And using self.__class__ can on the very rare occasion return the wrong result, so it is better to use type(x). Python2 docs as the Python3 docs don't state it as clearly. Also type(x) looks nicer than x.__class__.

type(x) is typically the same as x.__class__ (although this is not guaranteed – a new-style class instance is permitted to override the value returned for x.__class__).

So aside from nit-pics, I would: (With help from @holroy)

lst = [self.value, self.left, self.right]
if self.left or not self.right:
    lst = filter(lambda x: x is not None, lst)
return '{}({})'.format(type(self).__name__, ', '.join(map(repr, lst)))

The answer by Joe Wallis got me thinking about an option of just popping the last values of a list which would make for something like:

# Make a list matching the parameters of the __init__ method
lst = [self.value, self.left, self.right]

# Pop values from lst as long as they match the default value of None (or falsy)
while lst and not lst[-1]:
    lst.pop()

# Regenerate the call to the constructor
return '{}({})'.format(type(self).__name__, ', '.join(map(repr, lst)))

Which would pop values from the end as long as they're not returning a falsy value, which would work in my current case. However as I tend to want generic solutions (not directly asked for in the question), I found this alternate solution which would allow for the default values to be real default values, and not only None.

# Make a list matching the parameters of the __init__ method
lst = [self.value, self.left, self.right]

# Get all default values using introspection, and reverse list
default_values_reversed = reversed(getargspec(type(self).__init__).defaults)

# Pop values from lst as long as they match the default value
for default_value in default_values_reversed:
    if lst[-1] == default_value:
        lst.pop()
    else:
        break

# Regenerate the call to the constructor
return '{}{}'.format(type(self).__name__, repr(tuple(lst)))

##old: return '{}({})'.format(type(self).__name__, ', '.join(map(repr, lst)))

This version does indeed work, also with different types and values of default parameters. I do however not like that you'll manually have to specify the default parameters at the beginning, which I guess could be built using introspection as well (with some extra magic). Nor do I like the regeneration at the end. This applies to both these version, as well as the version suggested by Joe Wallis.

But it does work, and can absolutely be used in the generic case as well as in my specific case.

Added: I did however not really like the regeneration in Joe Wallis' version (or my first version in this answer), so after thinking a little on that, combined with the idea of calling repr() only once from Barry's solution, I changed the last line of the previous code block. Now I would only like to simplify the for-loop somehow... (and possibly autogenerate the argument list)

Addendum: Taking it to the extreme

Thanks to more hints from Joe Wallis, here is the extreme variant where the instance values, and default values are gathered through introspection, and through a bit of magic removed if the tailing end is equal using multiple reversed lists and itertools.dropwhile. Resulting in the following code:

from inspect import getargspec
from itertools import dropwhile

def __repr__(self):
    """Return a string which when eval'ed returns equal instance."""

    init_arg_spec = getargspec(type(self).__init__)

    # Make a list matching the parameters of the __init__ method
    instance_values_reversed = [self.__dict__[i]
                                for i in init_arg_spec.args[:0:-1]]

    # Make a reversed generator going through default values from inspection 
    default_values_reversed = reversed(init_arg_spec.defaults)

    init_args = tuple(list(dropwhile(
                             lambda val: val == next(default_values_reversed),
                             instance_values_reversed
                          ))[::-1])

    # Regenerate the call to the constructor
    return '{}{}'.format(type(self).__name__, repr(init_args))

This code works under the assumption that the instance variables have corresponding names to the constructor parameters, and that given an equal set of constructor parameters the instances are equal in value. If these requirements are met, this is a generic __repr__ method applicable to any given class.

Let's talk about this:

if (bool(self) ^ bool(other)
    or (bool(self.right) ^ bool(other.right))
    or (bool(self.left) ^ bool(other.left))):

The ^ operator is a bitwise operator. It only works in this case because True == 1 and False == 0, which could be confusing if the reader is thinking logically instead of numerically (i.e. "Wait, why are we doing bit fiddling here?"). There is no logical XOR operator because logical XOR is just !=. So you can rewrite this as follows:

if (bool(self) != bool(other)
    or (bool(self.right) != bool(other.right))
    or (bool(self.left) != bool(other.left))):

We still need the bool() calls because otherwise we'd be recursing here, which is not what we want to do.

bool(self) is True. If self were None, we would not have gotten this far, and we haven't overridden any magic methods which could make instances falsey. So we can change the first clause to True != bool(other) which is just a fancy way of writing not other (or other is None, if you prefer explicit None checks):

if (not other
    or (bool(self.right) != bool(other.right))
    or (bool(self.left) != bool(other.left))):

A variant over this answer form Gareth Rees, without repetition, but slightly more advanced if conditions:

def __repr__(self):
   fmt = '{}({value!r}'

   if self.left or self.right:
       fmt += ', {left!r}'

   if self.right:
       fmt += ', {right!r}'

   fmt += ')'

   return fmt.format(type(self).__name__, **vars(self))

Note that using !r calls repr() on the value (and !s, if used, calls str() on the value), according to Format string syntax. But Gareth is right that sometimes, simplicity is better than avoiding repetition.

A more compact version of this could be formed using join:

def __repr__(self):
    fmt = ''.join(('{}({value!r}',
                   ', {left!r}' if self.left or self.right else '',
                   ', {right!r}' if self.right else '',
                   ')'  ))

    return fmt.format(type(self).__name__, **vars(self))