1

I don't quite understand what is going with variable 'current_label', which to me seems to be defined in the enclosing code of function 'ts_r', which would make it visible from inside 'ts_r'? But when I run the code below, it complains that local variable 'current_label' is referenced before assignment... Note that it does not complain about 'visited' or 'f', and that it won't complain if I initialize 'current_label' with [len(g)].

def topological_sort(g):

    visited = zeros((len(g)), dtype='int32')
    f = zeros((len(g)), dtype='int32')
    current_label = len(g) # [] so it is seen inside ts_r

    def ts_r(n):
        for nn in [v for v in g[n] if not visited[v]]:
            visited[nn] = 1
            ts_r(nn)
        f[n] = current_label
        current_label -= 1

    for i in range(len(g)):
        if not visited[i]: 
            ts_r(i)

    return f
Frank
  • 4,341
  • 8
  • 41
  • 57

1 Answers1

3

In case of visited or f you change mutable variables. In case of current_label you try to re-assign value to global variable without stating it is global.

Changing variables from outside scopes does not require declaring them global, but reassigning value to a global variable requires declaration, that it is global - otherwise it is treated as local (and in case of referencing before assignement you get such errors).

Lets look at the code:

1.    def ts_r(n):
2.        for nn in [v for v in g[n] if not visited[v]]:
3.            visited[nn] = 1
4.            ts_r(nn)
5.        f[n] = current_label
6.        current_label -= 1

In line 5 you assign global variable value to f[n], but later, in line 6 you try to assign this global variable a value. You did not tell Python it is global, thus it assumes it is local. But if it is local, you cannnot assign it earlier.

You have two choices:

  1. (probably not the one you are looking for) Use it as local:

    def ts_r(n):
        current_label = len(g)  # initialize local variable
        for nn in [v for v in g[n] if not visited[v]]:
            visited[nn] = 1
            ts_r(nn)
        f[n] = current_label
        current_label -= 1
    
  2. Tell Python it is global variable and you would like to change global variable's value:

    def ts_r(n):
        global current_label  # current_label is now global
        for nn in [v for v in g[n] if not visited[v]]:
            visited[nn] = 1
            ts_r(nn)
        f[n] = current_label
        current_label -= 1
    

EDIT:

After your question was updated, I saw nested functions instead of functions defined in global scope. Thus the solution with global won't work.

In Python 3.x you have nonlocal keyword, but you will need to find walkaround in case of Python 2.x. Again, you have at least two possibilities:

  1. Use mutable variable enclosing immutable you want to change (eg. list with one integer in it). Then when you just refer to (and change) the first element of a list. Try it.

  2. Another solution is to add an attribute for the wrapping function (function is also a mutable, so you can change it, but you will not pollute global namespace). Example is here: http://ideone.com/7jGvM. In your case it may look like this:

    def topological_sort(g):
    
        visited = zeros((len(g)), dtype='int32')
        f = zeros((len(g)), dtype='int32')
        topological_sort.current_label = len(g) # [] so it is seen inside ts_r
    
        def ts_r(n):
            for nn in [v for v in g[n] if not visited[v]]:
                visited[nn] = 1
                ts_r(nn)
            f[n] = topological_sort.current_label
            topological_sort.current_label -= 1
    
        for i in range(len(g)):
            if not visited[i]: 
                ts_r(i)
    
        return f
    
Tadeck
  • 132,510
  • 28
  • 152
  • 198
  • Do you think its even necessary to have a local closure function in this situation in the first place? – jdi May 23 '12 at 01:32
  • @jdl: If you are referring to `ts_r`, then I do not have enough information. Maybe it is. Lets try to explain the error to OP. – Tadeck May 23 '12 at 01:33
  • Hmmmm... First, nothing in the syntax hints at current_label being of a different kind than f at the point of declaration. Why is current_label not considered a mutable variable too? - Second, if I don't want to pollute the global namespace, can I keep current_label's scope to be the whole of topological_sort, but not outside of it? – Frank May 23 '12 at 01:33
  • @Frank: type `int` is immutable. You keep reassigning it, as opposed to the others which are lists which you are only modifying by index. They stay the same object – jdi May 23 '12 at 01:35
  • Got it. I'll have to remember that type int is immutable. Remains the question about keeping current_label from spreading into the global namespace. By the way, I have to put a global declaration in topological_sort and another one in ts_r for the "global" thing to work. – Frank May 23 '12 at 01:37
  • @Frank: This is more in the way language works. Mutable are for example lists and dictionaries, but integers are not. I have just spotted (after someone has edited your question), that you have nested functions. In that case you can walk around this issue using this solution: http://stackoverflow.com/a/3190783/548696. Newer versions of Python (3+) have `nonlocal` statements, that allow you to do that without polluting global namespace. – Tadeck May 23 '12 at 01:37
  • @Tadeck: That was what I was referring to, when I suggested the local closure wasn't even necc, because it was just nested and called in a loop. – jdi May 23 '12 at 01:38
  • OK, I also discovered that I can use a [] instead of dictionary, putting current_label in []. It works and Python doesn't complain. – Frank May 23 '12 at 01:40
  • @jdi: the nested function is because I don't want to expose it outside topological sort, but I really need, because it does a recursive call. – Frank May 23 '12 at 01:41
  • @Frank: Gah sorry. My eyes didn't zero in on the recursive call. You are right. But, about the [int] thing...that really just feels like a bandaid approach as opposed to structuring it better. You will have to add the extra index lookup to each operation, instead of just decrementing a counter – jdi May 23 '12 at 01:42
  • @Tadeck: I like the solution to prefix with the name of the function. That's more "intuitive" than putting current_label in a dictionary or list, IMHO. Good stuff. – Frank May 23 '12 at 01:43