0

The following code does not work.

def set_view_counts(self):
    """
    Initializes the view counts for all of the Concept objects in the ConceptModel. See Concept for a
    description of why this parameter is optional.
    """
    for node in self.nodes():
        p = PageviewsClient().article_views("en.wikipedia", [node.concept.replace(' ', '_')])
        p = [p[key][node.concept.replace(' ', '_')] for key in p.keys()]
        p = int(sum([daily_view_count for daily_view_count in p if daily_view_count])/len(p))
        node.properties['view_count'] = p

When I check the contents of my node.properties dictionaries I find 4560, 4560, 4560, 4560.

The following code does.

def set_view_counts(self):
    """
    Initializes the view counts for all of the Concept objects in the ConceptModel. See Concept for a
    description of why this parameter is optional.
    """
    for node in self.nodes():
        p = PageviewsClient().article_views("en.wikipedia", [node.concept.replace(' ', '_')])
        p = [p[key][node.concept.replace(' ', '_')] for key in p.keys()]
        p = int(sum([daily_view_count for daily_view_count in p if daily_view_count])/len(p))
        node.properties = p

When I check properties I find 11252, 7367, 3337, 4560.

What's going on here?

Aleksey Bilogur
  • 3,686
  • 3
  • 30
  • 57
  • Might be just me or the reason is maybe quite simple considering that you're storing p in properties['view_count'] and not properties? – Torxed Dec 30 '15 at 05:00
  • Related brought me to http://stackoverflow.com/questions/38987/how-can-i-merge-two-python-dictionaries-in-a-single-expression?rq=1, which fixed the issue. But I still want to understand what the issue here is; I can tell that the problem is something about `self` being a temporary object, but I can't for the life of me see why. – Aleksey Bilogur Dec 30 '15 at 05:01
  • If this is a `self` related issues w would need all your code to see how you use `self.nodes` elsewhere – Torxed Dec 30 '15 at 05:03
  • My guess is that somewhere in your code you're done something like node1.properties = node2.properties = a dict or an int. This is ok in the latter case where node.properties is an int, but if node.properties is a dict, that statement make them point to the same dictionary, hence getting 4560 (latest assignment to the dictionary) for all of them. We can help you more if you share your whole code – kakhkAtion Dec 30 '15 at 05:19
  • It can only happen if you have a bug in the get/set method for the `properties` property. You need to show us the `node` object definition. – Burhan Khalid Dec 30 '15 at 05:21
  • Apologies for the late response, I am a novice with Git and managed to make quite a mess in my local repository. I've fixed it now, so you can find the code here: https://github.com/ResidentMario/watsongraph/blob/master/watsongraph/conceptmodel.py Line 134. – Aleksey Bilogur Dec 30 '15 at 05:24

1 Answers1

1

We would need to see more of your code, but I put some meat around your function, guessing what you could have written to reproduce your bug:

class Node:
    def __init__(self, props={}):
        self.properties = props

class G:
    def __init__(self):
        self.n = [Node(), Node(), Node(), Node()]

    def nodes(self):
        return self.n

    def set_view_counts(self):
        p = 0
        for node in self.nodes():
            node.properties['view_count'] = p
            p = p + 1

    def __repr__(self):
        r = ''
        for node in self.nodes():
            r += node.properties.__repr__()
        return r

g = G()
g.set_view_counts()
print g

With this, I get:

{'view_count': 3}{'view_count': 3}{'view_count': 3}{'view_count': 3}

This is because of the default value on the props parameter in Node.__init__. The same dict (the one used as the default value) is shared by all Nodes. Fix this by removing the default value.

Patrick Fournier
  • 600
  • 6
  • 19