sourcecred / research

Repository for research-related items on SourceCred's agenda
9 stars 7 forks source link

Lost utility function? #5

Closed mzargham closed 5 years ago

mzargham commented 5 years ago

bug nodePropertyDict(n)

not defined in: https://github.com/sourcecred/research/blob/sample-graphs/sample_graph_EDA/sample_graph_inspection_notebook.ipynb

it seems like it would have been defined in this cell:

def jsonToMultiDiGraph(json):
    [compat, data] = json
    assert compat["type"] == "sourcecred/graph"
    assert compat["version"] == "0.4.0"

    def propertyDict(address):
        # Note: This code "happens to work" for the sourcecred/git and sourcecred/github
        # plugins. However, it is not a requirement that e.g. the third element of the address
        # is a type, so this code should not be used in production or with general user-provided
        # plugins
        plugin = address[1]
        type = address[2]
        return {"address": tuple(address), "plugin": plugin, "type": type}

    nodes = data["nodes"]
    edges = data["edges"]
    g = nx.MultiDiGraph()
    for (i, n) in enumerate(nodes):
        g.add_node(i, **nodePropertyDict(n))
    for e in edges:
        g.add_edge(e["srcIndex"], e["dstIndex"], **propertyDict(e["address"]))
    return g
mzargham commented 5 years ago

Thanks @decentralion

teamdandelion commented 5 years ago

Yep, I originally had separate functions for node/edge property dict, but then collapsed them into one.

It would be great to have a CI tool that evaluates every cell and verifies that nothing throws an error like this. It's pretty easy to code bugs like this because the local kernel keeps the old symbols in scope.

I guess as a personal code review habit, I'll start restarting the kernel and then re-evaluating every cell before I send it in.

mzargham commented 5 years ago

My habit is always to hit the restart run all before commit. It can fail for lots of reasons, even just cell order, for the reason you mentioned.