Python: what's the pythonic way to perform this loop?

Question

What is the pythonic way to perform this loop. I'm trying to pick a random key that will return a subtree and not the root. Hence: 'parent == None' cannot be true. Nor can 'isRoot==True' be true.

thekey = random.choice(tree.thedict.keys())
while (tree.thedict[thekey].parent == None)or(tree.thedict[thekey].isRoot == True):
        thekey = random.choice(tree.thedict.keys())
.......

edit: it works now

Answer 1

break that while clause up into a few chunks, it will be easier to understand and debug.

Answer 2

thekey = random.choice(tree.thedict.keys())
parent = thedict[thekey].parent
while parent is None or parent.isRoot:
    thekey = random.choice(tree.thedict.keys())
    parent = thedict[thekey].parent

Answer 3

get a random subtree that is not the root

not_root_nodes = [key, node for key,node in tree.thedict.iteritems() if not ( node.parent is None or node.isRoot)]
item = random.choice( not_root_nodes )

Answer 4

key = random.choice([key for key, subtree in tree.thedict.items()
                         if subtree.parent and not subtree.isRoot])

(Corrected after comments and question edition)

Answer 5

I think that's a bit better:

theDict = tree.thedict

def getKey():
    return random.choice(theDict.keys())

theKey = getKey()

while theDict[thekey].parent in (None, True):
    thekey = getKey()

What do you think?

Answer 6

def is_root(v): 
  assert (v.parent != None) == (v.isRoot)
  return v.isRoot
  #note how dumb this function looks when you guarantee that assertion

def get_random_nonroot_key():
  while True:
    thekey = random.choice(tree.thedict.keys())
    value = tree.thedict[thekey]
    if not is_root(value): return key

or a refactoring of Roberto Bonvallet's answer

def get_random_nonroot_key():
  eligible_keys = [k for k, v in tree.thedict.items() if not is_root(v)]
  return random.choice(eligible_keys)

Answer 7

I think your while condition is flawed:

I think you expect this: tree.thedict[thekey].parent == None
should be equal to this: tree.thedict[thekey].parent.isRoot == True

When in fact, for both to mean "this node is not the root", you should change the second statement to: tree.thedict[thekey].isRoot == True

As written, your conditional test says "while this node is the root OR this node's parent is the root". If your tree structure is a single root node with many leaf nodes, you should expect an infinite loop in this case.

Here's a rewrite:

thekey = random.choice(k for k in tree.thedict.keys() if not k.isRoot)

Answer 8

thekey = random.choice(tree.thedict.keys())
parent = tree.thedict[thekey].parent
while parent is None or tree.thedict[thekey].isRoot:
    thekey = random.choice(tree.thedict.keys())
    parent = thedict[thekey].parent

Answer 9

Personally, I don't like the repetition of initializing thekey before the while loop and then again inside the loop. It's a possible source of bugs; what happens if someone edits one of the two initializations and forgets to edit the other? Even if that never happens, anyone reading the code needs to check carefully to make sure both initializations match perfectly.

I would write it like so:

while True:
    thekey = random.choice(tree.thedict.keys())
    subtree = tree.thedict[thekey]
    if subtree.parent is not None and not subtree.isRoot:
        break

P.S. If you really just want the subtree, and don't care about the key needed to lookup the subtree, you could even do this:

while True:
    subtree = random.choice(tree.thedict.values())
    if subtree.parent is not None and not subtree.isRoot:
        break

Some people may not like the use of "while True:" but that is the standard Python idiom for "loop forever until something runs break". IMHO this is simple, clear, idiomatic Python.

P.P.S. This code should really be wrapped in an if statement that checks that the tree has more than one node. If the tree only has a root node, this code would loop forever.